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 function to carry max_history to featurizer #8292

Merged

Conversation

joaoCeilandia
Copy link
Contributor

@joaoCeilandia joaoCeilandia commented Mar 24, 2021

Proposed changes:

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 TyDunn March 25, 2021 07:00
@sara-tagger
Copy link
Collaborator

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

@TyDunn TyDunn requested review from Ghostvv and removed request for TyDunn March 25, 2021 12:44
joaoCeilandia and others added 2 commits March 25, 2021 14:22
change the way to carry max_history to featurizer

Co-authored-by: Vladimir Vlasov <[email protected]>
remove unused function in the solution
@Ghostvv Ghostvv enabled auto-merge (squash) March 25, 2021 17:55
@Ghostvv Ghostvv disabled auto-merge March 25, 2021 17:55
@joaoCeilandia
Copy link
Contributor Author

@Ghostvv sorry for requiring that much from you but do you face that fail before? Could you give me a hint?
image

@Ghostvv
Copy link
Contributor

Ghostvv commented Mar 26, 2021

@wochinge do you have an idea why docs test is failing?

@HotThoughts
Copy link
Contributor

@wochinge do you have an idea why docs test is failing?

Hey @joaoCeilandia and @Ghostvv, the docs test just fixed! When you merge the main branch into this PR, the docs test should be green.

@joaoCeilandia
Copy link
Contributor Author

image

@Ghostvv do you know if this file has been modified or updated to a new version in the current rasa version? If not, do you know how to solve this?

@Ghostvv
Copy link
Contributor

Ghostvv commented Apr 9, 2021

I don't know. @wochinge or @joejuzl could you please take a look at that failing test

@joaoCeilandia
Copy link
Contributor Author

@Ghostvv the failing test disappeared, it's abble to merge now.

@ErickGiffoni ErickGiffoni deleted the 5786-max_history-parameter-should-be-carried branch August 5, 2021 19:42
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.

Max_history parameter should be carried over to the featurizer definition in TEDPolicy
4 participants