Update rules to allow more variation for MonthDDDD intervals. #271
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
| @@ -1118,6 +1118,30 @@ ruleIntervalFromDDDDMonth = Rule | |||
| _ -> Nothing | |||
| } | |||
|
|
|||
| ruleIntervalFromDDDDOfMonth :: Rule | |||
There was a problem hiding this comment.
Added this rule because it was failing too early when trying to add "of" in the above rule.
There was a problem hiding this comment.
This is the right way to go. ruleIntersectOf might get in the way when disambiguating but adding more corpus examples should help. Make sure to re-generate the classifiers with the latest code and include them in the commit.
Duckling/Time/EN/Corpus.hs
Outdated
| @@ -958,6 +958,10 @@ allExamples = concat | |||
| , "from July 13-15" | |||
| , "from 13 to 15 July" | |||
| , "from 13th to 15th July" | |||
| , "from the 13th to 15th July" | |||
| , "from the 13th to the 15th July" | |||
| , "from the 13th to the 15th of July" | |||
There was a problem hiding this comment.
Fails here. Trying to figure out what I'm doing wrong in the rule.
| , Predicate isDOMValue | ||
| , regex "\\-|to( the)?|th?ru|through|(un)?til(l)?" | ||
| , Predicate isDOMValue | ||
| , regex "of" |
There was a problem hiding this comment.
For some reason, I am unable to get this to work for "of".
There was a problem hiding this comment.
It's because when pattern matching tokens in the production rule, you're missing the new regex match token.
| token1: | ||
| _: | ||
| token2: | ||
| Token Time td: |
There was a problem hiding this comment.
| Token Time td: | |
| _: | |
| Token Time td: |
There was a problem hiding this comment.
That was it. Thank you @patapizza for spotting that. Makes a lot more sense now 👍
MorrisLaw
changed the title
There was a problem hiding this comment.
@patapizza has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Thank you @MorrisLaw! |
Fixes #218
Note: This is still a work in progress. But, I'm new to both Haskell and this repo and I seem to be stuck. Would appreciate feedback on what I've done so far. Thank you!