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

Fix duplicate interactive learning events #3719

Merged
merged 12 commits into from
Jun 11, 2019

Conversation

wochinge
Copy link
Contributor

@wochinge wochinge commented Jun 6, 2019

Proposed changes:

Status (please check what you already did):

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

@wochinge wochinge force-pushed the fix-duplicate-interactive-learning-events branch 2 times, most recently from 7c1ff86 to 18524d7 Compare June 6, 2019 13:47
@wochinge wochinge force-pushed the fix-duplicate-interactive-learning-events branch from 18524d7 to 92d9428 Compare June 6, 2019 13:52
@wochinge wochinge marked this pull request as ready for review June 6, 2019 13:53
@wochinge wochinge requested review from tabergma and removed request for tabergma June 6, 2019 15:04
@wochinge wochinge requested a review from tabergma June 6, 2019 15:36
Copy link
Contributor

@tabergma tabergma left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for cleaning up 🚀

Left just a few minor comments.

rasa/core/training/interactive.py Outdated Show resolved Hide resolved
@@ -424,6 +416,7 @@ def _selection_choices_from_intent_prediction(
choices, sender_id, endpoint
)

# TODO: Should we not always set the confidence = 1.0?
Copy link
Contributor

Choose a reason for hiding this comment

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

Which scenarios do you mean? The scenario in which the user selected a specific intent? Maybe, something to discuss in Slack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed it with Akela, and we now set it to 1.0

rasa/core/training/interactive.py Outdated Show resolved Hide resolved
@wochinge
Copy link
Contributor Author

wochinge commented Jun 7, 2019

implemented the review comments 👍

@wochinge wochinge merged commit daeb88d into master Jun 11, 2019
@wochinge wochinge deleted the fix-duplicate-interactive-learning-events branch June 11, 2019 08:36
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.

Duplicate event in SQLTracker
2 participants