Change the location.rx files to be less greedy. They were conflictin…

[chrisveilleux]

trafficstars

Description

Change the location.rx files to be less greedy. They were conflicting with the location.rx files in other skills.

If needed follow up with as much detail as required.

Type of PR

If your PR fits more than one category, there is a high chance you should submit more than one PR. Please consider this carefully before opening the PR. Either delete those that do not apply, or add an x between the square brackets like so: - [x]

• [X] Bugfix
• [ ] Feature implementation
• [ ] Refactor of code (without functional changes)
• [ ] Documentation improvements
• [ ] Test improvements

Testing

This change should be a no-op for this skill so VK tests should pass.

Documentation

Not needed for this PR

[krisgesling]

With this change the VK tests aren't passing for me.

The description somewhat concerns me though... Does this mean that if I load another Skill that has a location.rx file registered with Adapt that it might impact the behaviour of the location regex in this Skill?

If so, has this been raised as an issue in the Adapt repo already? It seems very problematic.

[forslund]

The issue here is the .* in the beginning will cause it to generate a lot of extra matches making the regex parsing in adapt heavier and giving lower scores. it should not be there, so to me this is definitely a good change if the VK tests can be made to pass...

I also think the * at the end in *.+ can be removed as well since it will likely always be matched against the empty sequence (If I'm reading the regex correctly, which is definitely not certain)

General rule for adapt regexes In general .+ (match at least one any character) is preferable over .* (match at least zero any character)

[krisgesling]

I'm getting much more consistent behaviour with the recent changes. Though this might be false optimism...

I am still getting an issue with the test for these two utterances:

temperature in the afternoon what is the weather in London later today

and it seems to be due to this location regex.

At a more foundational level, it seems like a literal keyword match should be given more weight than a patterned regex match. Eg afternoon.voc should get a higher confidence level than this location regex.