-
Notifications
You must be signed in to change notification settings - Fork 91
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
Better examples of using Intents with Adapt #42
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great addition! Most uses of "Location" use capital L, but line 446 cites file name with lower case. Is that correct? Does this illustrate some case insensitivity?
Any guidance on how many words (and/or types of words) not represented in the intent elements can appear between represented words? Including but not limited to "unnecessary" adjectives or grammar parts of speech.
Thanks for including reference url at line 432 for re. I think it might yet be helpful to just mention that the regex uses python specific named patterns. Of course someone who cares will figure it out, but you do get the occasional person familiar with regex, but new to python.
Thanks for your feedback @jrwarwick, I've added another commit which I think addresses some of your feedback. What do you think? |
I'm going to merge this for better or worse, I can always do more edits. |
Kathy , thanks and sorry for late reply. I think this is definitely an improvement. One thing I'm still not clear about (so maybe just my problem) is how the filename on line 446 relates, or maybe it does not? I believe that with .voc vocab files, the first part of the filename is what appears in the .require() method call argument. But perhaps it is different for regex, and instead of regex filename, the argument represents the Regex group name. So in that case is it correct that when the skill is loaded, all files in regex directory are scanned (filenames don't really matter, just for human understanding) and a list of all regex pattern names is extracted for matching the require() method call's argument. Is that right? |
Based on conversations with @penrods, this covers
I'm still not 100% happy with it, but it's better than what we have.
Assigning to @penrods for review (sorry to throw more at you)