rasa icon indicating copy to clipboard operation
rasa copied to clipboard

Published 20 hours ago • verified publisher icon RasaHQ

Rename EntityExtractorMixin

Open VitorLamego opened this issue 1 year ago • 14 comments

Co-authore-by: AlGouvea [email protected]

Proposed changes:

  • As proposed in the https://github.com/RasaHQ/rasa/issues/10225 references of EntityExtractorMixin were renamed to EntityExtractor for consistency.

Status (please check what you already did):

  • [x] added some tests for the functionality
  • [x] updated the documentation
  • [x] updated the changelog (please check changelog for instructions)
  • [x] reformat files using black (please check Readme for instructions)

VitorLamego avatar Jul 18 '22 22:07 VitorLamego

@VitorLamego I'm adding one thing, which is a backwards compatibility EntityExtractorMixin class that will emit a deprecation warning when used. The reason for this is to prevent breaking changes in a minor.

indam23 avatar Aug 03 '22 11:08 indam23

Since I can't push to your fork, there's a branch & commit here with the suggested changes - could you pull these into your branch? https://github.com/RasaHQ/rasa/commit/b1401df095049b04c230fbf0005a306ee39eb144

indam23 avatar Aug 03 '22 12:08 indam23

Done!

VitorLamego avatar Aug 04 '22 19:08 VitorLamego

Thanks, do you mind addressing the failing CI checks ? They're my fault, I added the logging module when testing locally. The documentation check is failing because of a flaky link, don't worry about that one.

indam23 avatar Aug 04 '22 21:08 indam23

No problem! Is there any way I could help on documentation check ?? @melindaloubser1

VitorLamego avatar Aug 09 '22 14:08 VitorLamego

There was a PR to make the check non-required here: https://github.com/RasaHQ/rasa/pull/11413 - the commit is also on main, so if you merge main into your branch, it should take care of it.

indam23 avatar Aug 09 '22 14:08 indam23

Last failing check here https://github.com/RasaHQ/rasa/runs/7748482684?check_suite_focus=true you can fix it by running make formatter

indam23 avatar Aug 10 '22 12:08 indam23

@VitorLamego I'm so sorry, now there's conflicts that have to be resolved - I'll try make sure this time once that's done this gets merged quickly so it doesn't get into a conflict state again

indam23 avatar Aug 15 '22 15:08 indam23

Done @melindaloubser1 !

VitorLamego avatar Aug 16 '22 01:08 VitorLamego

Everything ok ? @melindaloubser1

VitorLamego avatar Aug 18 '22 15:08 VitorLamego

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 11 '22 04:09 CLAassistant

Sorry for the delay! I believe it is ok now :) @ancalita

VitorLamego avatar Sep 11 '22 04:09 VitorLamego

It seems that errors on CI are not due to this issue, right ?? Anything else we could do to help ? @ancalita

VitorLamego avatar Sep 14 '22 14:09 VitorLamego

It seems that errors on CI are not due to this issue, right ?? Anything else we could do to help ? @ancalita

I've restarted the jobs, could have been flaky incidents.

ancalita avatar Sep 21 '22 15:09 ancalita