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

Dialogflow Import Fix for Issue 6137 #6138

Closed
wants to merge 4 commits into from
Closed

Conversation

A-A-R0N
Copy link
Contributor

@A-A-R0N A-A-R0N commented Jul 3, 2020

Proposed changes:

  • Fixed passing the wrong value to get_nlu_data during training

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)

  • This is a fix for existing functionality which I tested against several dialogflow models.

  • No documentation are updates. The steps in current documentation work as expected now.

@CLAassistant
Copy link

CLAassistant commented Jul 3, 2020

CLA assistant check
All committers have signed the CLA.

@sara-tagger
Copy link
Collaborator

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

@sara-tagger sara-tagger requested a review from indam23 July 6, 2020 06:00
@indam23
Copy link
Contributor

indam23 commented Jul 8, 2020

Thanks for the PR! I don't have a dialogflow bot to test this with, but what happens now if the language is not defined in the config or on import? It still throws an error right?
When is the language getting overwritten and why?

Edit: Sorry I just missed the linked issue! I'll look at that

@indam23 indam23 changed the base branch from master to 1.10.x July 10, 2020 13:12
@indam23 indam23 changed the base branch from 1.10.x to master July 10, 2020 13:13
Copy link
Contributor

@indam23 indam23 left a comment

Choose a reason for hiding this comment

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

Thanks for noticing this! For it to go into a patch, it needs to be based off of 1.10.x, not master - do you mind rebasing it?

@A-A-R0N
Copy link
Contributor Author

A-A-R0N commented Jul 10, 2020

@melindaloubser1 I recreated this as PR 6188. This way I don't have to worry about any previous merge conflicts that have already been resolved.

@indam23
Copy link
Contributor

indam23 commented Jul 10, 2020

Sure, that makes sense

@indam23 indam23 closed this Jul 10, 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.

Dialogflow import bug "No training examples found for the dialogflow file data\intents\<every intent>.json!"
4 participants