Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly set slots in domain when just updating nlg. #7417

Merged
merged 2 commits into from
Dec 1, 2020

Conversation

joejuzl
Copy link
Contributor

@joejuzl joejuzl commented Nov 30, 2020

Proposed changes:
Domain.compare_with_specification was failing when running rasa train after only changing NLG data. This is because the categorical slots were not having the default __other__ value added when the domain was updated using update_model_with_new_domain - so it ended up being inconsistent.
Solution was to also set up the default slots in this method.

Status (please check what you already did):

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

@joejuzl joejuzl requested a review from a team as a code owner November 30, 2020 16:23
@joejuzl joejuzl requested review from alwx and removed request for a team November 30, 2020 16:23
@Ghostvv Ghostvv added the type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. label Nov 30, 2020
Copy link
Contributor

@Ghostvv Ghostvv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checked, works for me

@joejuzl joejuzl force-pushed the 7322/correctly_update_domain_if_no_ml_training branch from 45742c0 to f1dc526 Compare December 1, 2020 08:47
@joejuzl joejuzl changed the base branch from master to 2.1.x December 1, 2020 08:48
Copy link
Contributor

@alwx alwx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@joejuzl joejuzl merged commit 9524be1 into 2.1.x Dec 1, 2020
@joejuzl joejuzl deleted the 7322/correctly_update_domain_if_no_ml_training branch December 1, 2020 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants