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

Add split_entities_config to TEDPolicy #7716

Merged
merged 17 commits into from
Jan 25, 2021
Merged

Add split_entities_config to TEDPolicy #7716

merged 17 commits into from
Jan 25, 2021

Conversation

koernerfelicia
Copy link
Contributor

@koernerfelicia koernerfelicia commented Jan 13, 2021

Proposed changes:
Closes #7707

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)

docs/docs/policies.mdx Show resolved Hide resolved
rasa/core/policies/ted_policy.py Outdated Show resolved Hide resolved
@koernerfelicia koernerfelicia added the tools:clear-poetry-cache-unit-tests Clear poetry cache for the unit tests label Jan 18, 2021
@koernerfelicia
Copy link
Contributor Author

@tttthomasssss and @Ghostvv, I applied the suggestions. One thing I'm not so sure whether ResponseSelector should have a default value for split_entities_config

@Ghostvv
Copy link
Contributor

Ghostvv commented Jan 20, 2021

ResponseSelector doesn't extract entities, so I'd say that it shouldn't have it. @tttthomasssss could you please review the PR, since the original config is yours

@tttthomasssss
Copy link
Contributor

@koernerfelicia great work 🚀 !

I've left a few comments.

Base automatically changed from master to main January 22, 2021 11:15
Copy link
Contributor

@tttthomasssss tttthomasssss left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for addressing my comments - LGTM 👍 !

@koernerfelicia koernerfelicia merged commit 7352104 into main Jan 25, 2021
@koernerfelicia koernerfelicia deleted the iss-7707 branch January 25, 2021 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools:clear-poetry-cache-unit-tests Clear poetry cache for the unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add split_entities_config to TEDPolicy for e2e
3 participants