Skip to content
This repository was archived by the owner on Sep 8, 2024. It is now read-only.

Intent Parser helper class #859

Closed
wants to merge 11 commits into from
Closed

Intent Parser helper class #859

wants to merge 11 commits into from

Conversation

@JarbasAI
Copy link
Contributor

@JarbasAI JarbasAI commented Jun 29, 2017

helper class to query intent service instance

splitting #783 in smaller PRs

helper class to be imported and used in other skills

from mycroft.skills.intent_service import IntentParser

Parser = IntentParser(self.emitter)

intent_name = "HelloWorldIntent"
list_of_skills_with_this_intent = Parser.get_skill_from_intent(intent_name)

utterance = "Hello world"
skill_utterance_will_trigger = Parser.get_skill_from_utterance(utterance)
intent_utterance_will_trigger = Parser.get_intent(utterance, lang="en-us")

@JarbasAl
Intent Parser class
bb473f5 
helper class to query intent service instance
@penrods penrods requested a review from forslund July 6, 2017 07:28
forslund
forslund reviewed Jul 6, 2017
View reviewed changes
mycroft/skills/intent_service.py
new_parsers = [
p for p in self.engine.intent_parsers if
not p.name.startswith(skill_name)]
self.engine.intent_parsers = new_parsers
Copy link
Collaborator

@forslund forslund Jul 6, 2017

Choose a reason for hiding this comment

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

Shouldn't this remove the intent from the dictionary of skills and intents?

Copy link
Collaborator

@forslund forslund Jul 25, 2017

Choose a reason for hiding this comment

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

@JarbasAI any comment on this?

Copy link
Contributor Author

@JarbasAI JarbasAI Aug 17, 2017

Choose a reason for hiding this comment

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

i missed this comment, will add the remove functionality, perhaps i should wait on the converse PR since skill name will become skill_id ?

@forslund
Copy link
Collaborator

forslund commented Jul 6, 2017

As far as I can understand this it looks quite good. One small question that I'd like answered or fixed if it's an actual issue (see comment in code) and fix the pep8 problems and I think this is good to merge.

jarbasai added 2 commits July 18, 2017 16:56
@coveralls
Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage increased (+1.1%) to 41.123% when pulling 77d86f9 on JarbasAI:patch-18 into b98b020 on MycroftAI:dev.

@coveralls
Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage increased (+1.1%) to 41.123% when pulling 6b1627a on JarbasAI:patch-18 into b98b020 on MycroftAI:dev.

@coveralls
Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage increased (+1.1%) to 41.13% when pulling 6b1627a on JarbasAI:patch-18 into b98b020 on MycroftAI:dev.

@penrods penrods added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Aug 17, 2017
@penrods
Copy link
Contributor

penrods commented Aug 17, 2017

Converse() PR #925 is in! Using the skill ID is a much better approach, I agree.

jarbasai added 2 commits August 17, 2017 19:17
@coveralls
Copy link

coveralls commented Aug 17, 2017

Coverage Status

Coverage decreased (-0.4%) to 38.207% when pulling aefde61 on JarbasAI:patch-18 into d661b8d on MycroftAI:dev.

@coveralls
Copy link

coveralls commented Aug 17, 2017

Coverage Status

Coverage decreased (-0.4%) to 38.207% when pulling aefde61 on JarbasAI:patch-18 into d661b8d on MycroftAI:dev.

@coveralls
Copy link

coveralls commented Sep 2, 2017

Coverage Status

Coverage decreased (-0.3%) to 40.613% when pulling 6a82bb9 on JarbasAI:patch-18 into 66453cb on MycroftAI:dev.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage increased (+1.8%) to 42.648% when pulling 9e0db27 on JarbasAI:patch-18 into 66453cb on MycroftAI:dev.

@MycroftAI MycroftAI deleted a comment Sep 19, 2017
@JarbasAl
Copy link
Contributor

JarbasAl commented Nov 28, 2017

this one kinda got lost, now that we have padatious should i make it also get intents from there? how do i differentiate in the returned data?

@JarbasAI
Copy link
Contributor Author

JarbasAI commented Feb 7, 2018

should i update this for #1351 ?

@NeonJarbas NeonJarbas mentioned this pull request Feb 3, 2020
Merged
@JarbasAl JarbasAl mentioned this pull request Feb 3, 2020
Merged
@JarbasAl JarbasAl closed this Feb 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@forslund forslund forslund left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JarbasAI @forslund @coveralls @penrods @JarbasAl