search

MycroftAI / adapt

Adapt Intent Parser
Apache License 2.0
710 stars 154 forks source link

Add support for dropping things #122

Closed forslund closed 3 years ago

forslund commented 3 years ago

This adds support for removing intent parsers, registered entities (keywords) and regex entities.

The drop intent parsers has been done in Mycroft-core for years, and to make skill reloading more reliable I recently proposed dropping the registered vocabs as well.

This implements both these features in adapt which seems to be a more suitable place for it.

forslund commented 3 years ago

I've pushed the first pass of the rewrite, still need to write more tests and actually evaluate the drop_entity method.

I will do a second pass on the "reverse_search" method of the trie as well. feels like it can be made cleaner still.

clusterfudge commented 3 years ago

Cool, this is looking pretty good so far! I might consider renaming reverse_search as something like scan or just traverse, as it's more consistent with either a database or generic tree operation.

forslund commented 3 years ago

Will incorporate that name suggestion.

forslund commented 3 years ago

Ok I think I'm done messing around with this. It now supports dropping regexes, intents and normal entities.

I've tested the changes in Mycroft-Core, and things seem to work as I'd expect them to.

I bumped the version to 0.4.0 since it's new functionality.

forslund commented 3 years ago

Since I can't apparently be trusted to do even a 1 line method without typos and missing arguments I've added test cases for the Domain specific intent engine as well.

The usecase for the match functions is to "simply" be able to remove multiple keywords without traversing the tree many times (in Mycroft an example is when a skill is removed and all keyword entities from that skill is to be removed, in mycroft the entities are prepended with the skill name when registered to group them so something similar to a .startswith(skill_id) check is done to filter out the keyword entities of interest).

A possible alternative is to add another method get_entities() returning a list of all entities registered in the trie. The caller can search the list for the nodes matching the set of keywords. the drop_entity() can then be called with a list of entities...

But to me a simple match function / lambda would be tidier.

JarbasAl commented 3 years ago

i agree a simple match function / lambda is tidier