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.

Sign up for GitHub

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

Jump to bottom

Bugfix nlu comparison persist/load nlg responses #5202

Closed
wants to merge 71 commits into from
Closed

Bugfix nlu comparison persist/load nlg responses #5202

wants to merge 71 commits into from

Conversation

indam23
Copy link
Contributor

@indam23 indam23 commented Feb 7, 2020 •
edited
Loading

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)

indam23 and others added 16 commits February 3, 2020 10:18
@indam23
how to deploy docs page
c2088e0
@indam23
how to deploy docs page
6697d7b
@indam23
Merge branch 'how_to_deploy' of github.com:RasaHQ/rasa into how_to_de…
33946ca 
…ploy
@indam23 @TyDunn
Update docs/user-guide/how-to-deploy.rst
2ffa102 
Co-Authored-By: Ty Dunn <t.dunn@rasa.com>
@indam23 @akelad
Update docs/user-guide/how-to-deploy.rst
197a6f7 
Co-Authored-By: Akela Drissner-Schmid <32450038+akelad@users.noreply.github.com>
@indam23 @akelad
Update docs/user-guide/how-to-deploy.rst
f98aa95 
Co-Authored-By: Akela Drissner-Schmid <32450038+akelad@users.noreply.github.com>
@indam23 @akelad
Update docs/user-guide/how-to-deploy.rst
f9fa05e 
Co-Authored-By: Akela Drissner-Schmid <32450038+akelad@users.noreply.github.com>
@indam23
how to deploy doc updates
76a75dc
@indam23
merge with origin
a3af5db
@indam23 @TyDunn
Update docs/user-guide/how-to-deploy.rst
7f088b9 
Co-Authored-By: Ty Dunn <t.dunn@rasa.com>
@akelad
make integer primary key compatible with oracle
712d171
@akelad
add docs for oracle tracker store
deb5218
@akelad
add to changelog
96d5218
@tabergma
Add check for text features
a24376a
@akelad
fix SQL highlighting
acdddd4
@indam23
persist nlg responses along with nlu data for nlu pipeline comparison
5adbca8
@indam23 indam23 requested a review from dakshvar22 February 7, 2020 14:42
@indam23 indam23 changed the base branch from master to 1.7.x February 7, 2020 14:43
dakshvar22
dakshvar22 approved these changes Feb 7, 2020
View reviewed changes
Copy link
Contributor

@dakshvar22 dakshvar22 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! 🚀 Thanks for fixing it!
Minor: Not sure, but test_nlu_comparison test might fail. Can you check that once? Can we make it robust to catch this bug if it reappears anytime?

@indam23
Copy link
Contributor Author

indam23 commented Feb 7, 2020 •
edited
Loading

@dakshvar22 test_nlu_comparison passes; should I edit that test to include NLG responses or add a separate test?

*edit: It passes, but it wasn't checking if model files were actually created. Edited it so it does and also uses one pipeline with a response selector.

@indam23 indam23 requested a review from dakshvar22 February 7, 2020 21:05
indam23 and others added 3 commits February 7, 2020 16:10
@indam23
add data for test
173475a
@tabergma
Merge pull request #5200 from RasaHQ/fix-check-test-features
7ae4f4f 
Add check for text features
@akelad @wochinge
Apply suggestions from code review
b20dbf2 
Co-Authored-By: Tobias Wochinger <t.wochinger@rasa.com>
wochinge and others added 25 commits February 12, 2020 13:31
@wochinge
black formatting
28ec6c5
@akelad
update docstring
0cbc70d
@wochinge
Merge pull request #5222 from RasaHQ/fix-urlsplit-python3.7.6
79a5ffd 
Fix urlsplit python3.7.6
@akelad
Merge branch '1.7.x' into oracle-compatibility
2e9f490
@akelad
fix reference to running rasa with docker
56c5152
@akelad
update database compatibility docs
03e6134
@akelad
Merge pull request #5197 from RasaHQ/oracle-compatibility
8647eb7 
make integer primary key compatible with oracle
@akelad
Merge branch '1.7.x' into rename_running_server_rebase
73c9235
@akelad
update changelog
8ba2b4f
@akelad
Merge pull request #5210 from RasaHQ/rename_running_server_rebase
091bd0c 
Rename Running the Server to Configuring the HTTP API
@akelad
prepared release of version 1.7.2
b85ffc9
@akelad
fix oracle changelog entry
91d699a
@akelad
Merge pull request #5237 from RasaHQ/prepare-release-1.7.2
caf96f6 
Prepare release 1.7.2
@akelad
fix link to invite users to test
5309094
@erohmensing
Merge pull request #5242 from RasaHQ/fix_docs_link
086c724 
fix link to invite users to test
@wochinge
pin gym version to avoid segmentation fault
dfea4aa
@wochinge
remove unused import
02f6d83
@wochinge
add changelog
b7558ad
@tmbo
Merge pull request #5248 from RasaHQ/pin-gym
3d8fa7f 
Pin gym
@indam23
persist nlg responses along with nlu data for nlu pipeline comparison
d464ebd
@indam23
changelog for 5201 bugfix
4ca0b5b
@indam23
add to pytest of nlu comparison to ensure model files are actually cr…
4809e85 
…eated
@indam23
add data for test
8288676
@indam23
make data path a constant variable
1520976
@indam23
Merge branch 'bug_nlu_comparison_nlg_responses' of github.com:RasaHQ/…
e1cbd8a 
…rasa into bug_nlu_comparison_nlg_responses
@degiz degiz added this to the Rasa 1.8 milestone Feb 20, 2020
@indam23 indam23 mentioned this pull request Feb 20, 2020
Closed
4 tasks
@degiz degiz removed this from the Rasa 1.8 milestone Feb 20, 2020
@indam23
Copy link
Contributor Author

indam23 commented Feb 21, 2020

replaced by #5294

@indam23 indam23 closed this Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dakshvar22 dakshvar22 dakshvar22 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NLG responses not found during NLU pipeline comparison
8 participants
@indam23 @dakshvar22 @degiz @akelad @tabergma @tmbo @wochinge @erohmensing