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

Use NaturalLanguageInterpreter factory in agent internal method #6274

Closed
wants to merge 1 commit into from
Closed

Use NaturalLanguageInterpreter factory in agent internal method #6274

wants to merge 1 commit into from

Conversation

pheel
Copy link
Contributor

@pheel pheel commented Jul 24, 2020

Proposed changes:

  • Refactored agent internal method to use NaturalLanguageInterpreter factory instead of creating class directly.

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)

@sara-tagger sara-tagger requested a review from erohmensing July 27, 2020 06:00
@sara-tagger
Copy link
Collaborator

Thanks for submitting a pull request 🚀 @erohmensing will take a look at it as soon as possible ✨

@erohmensing
Copy link
Contributor

Hey @pheel, can you elaborate on how your change improves upon the existing code? it seems that the create method will end up instantiating a RasaNLUInterpreter in a few calls in any case.

@pheel
Copy link
Contributor Author

pheel commented Jul 28, 2020

@erohmensing I agree this is a rather inconsequential change on its own... This was done in the context of making UnsupportedModelError non-fatal (5faa4bb). Maybe it makes more sense to move this commit over there?

@erohmensing
Copy link
Contributor

Makes sense to me!

@pheel
Copy link
Contributor Author

pheel commented Jul 28, 2020

Gotcha. Moved to #6276.

@pheel pheel closed this Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants