-
Notifications
You must be signed in to change notification settings - Fork 155
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
Bug causing .optionally regex to not execute, but it works with .required. IntentBuilder #115
Comments
Would you mind creating this issue on the Adapt issue page instead? Also, it sounds similar, but the opposite to this issue. |
Hey, sorry for the super delayed response here. I believe that this issue was addressed by a bug in from adapt.intent import IntentBuilder
from adapt.engine import IntentDeterminationEngine
def run_test(required):
engine = IntentDeterminationEngine()
engine.register_entity("Test", "TestKeyword")
engine.register_regex_entity(r"by (?P<Artist>.*)")
test_intent = IntentBuilder("TestIntent"). \
require("TestKeyword")
if required:
test_intent = test_intent.require('Artist').build()
else:
test_intent = test_intent.optionally('Artist').build()
engine.register_intent_parser(test_intent)
print(list(engine.determine_intent("test by Artist")))
run_test(True)
run_test(False)
> [{'intent_type': 'TestIntent', 'TestKeyword': 'Test', 'Artist': 'artist', 'target': None, 'confidence': 0.375}]
> [{'intent_type': 'TestIntent', 'TestKeyword': 'Test', 'Artist': 'artist', 'target': None, 'confidence': 1.0}] |
I'm still experiencing this intermittently with adapt-parser 0.4.0. Here's my code (
And here's the regex file I'm using,
And that gives me the following output:
Then I tried it with this decorator and it worked, but I don't see why:
Is this user error? What can I do to help debug this? Is adapt being deprecated in favor of padatious? |
Is |
Ah yes, that is user error then, that's part of I still don't understand why it works now, though. |
hey @david-morris , I've opened MycroftAI/mycroft-core#2883 against mycroft to request some better tools to help us debug issues like this. Regex entities in particular are thorny in how they influence ranking, as are |
Hey folks, I'm facing a very similar problem and I can't figure how to fix. I have a |
Hi @oblitum , Additionally, as someone new to this community, I'm a bit uncertain about whether we should be posting on Adapt issues when mycroft-core currently uses adapt 0.3.7 and adapt 0.4.0 has been released. Could someone with more experience comment on that? |
Good question, I'm not sure what is best. maybe it's better to open on mycroft-core and if it can be determined that it's adapt and not mycroft-core that is at fault it can be moved by maintainers... But if the issue / question is specific to adapt it may be better to go here directly. So I'd say...it depends :P |
@david-morris thanks for your input regarding the docs, but I don't think it's a match issue, because actually the one I posted formerly was just one of the attempts, originally it was |
I've given up trying to use |
@oblitum me too. I wrote a method that's been merged into cpkodi for doing this consistently with multiline .rx files. |
@oblitum and @david-morris , in a lengthy explanation of some dark corners of adapt to @chrisveilleux , I concluded that there is a failure to meet the principle of least astonishment. I'm finally tackling this with #137 (and fixing a few other latent bugs), and I'd encourage you to try the latest version when it gets merged! |
I've observed that when doing a relatively simple regex with the IntentBuilder, the regex doesn't match when I include it with .optionally, but it works with .required.
This is most easily demonstrated with this example code: https://github.com/InconsolableCellist/test-skill/tree/optionally_bug
Steps to reproduce:
test a by artist
test a by artist
againExpected behavior
The skill prints an INFO message: "Artist found! artist" on both steps 4 AND 6
Observed behavior
The INFO message is only printed on step 4, not on step 6, indicating that the regex doesn't seem to match when you build the intent by optionally including the regex rather than requiring it.
The text was updated successfully, but these errors were encountered: