Skip to content
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

incorect date parsing: 31st Oct parsed as 1st Oct #34

Closed
chris48s opened this issue Jul 13, 2018 · 6 comments
Closed

incorect date parsing: 31st Oct parsed as 1st Oct #34

chris48s opened this issue Jul 13, 2018 · 6 comments
Labels

Comments

@chris48s
Copy link
Contributor

chris48s commented Jul 13, 2018

example: DemocracyClub/EveryElection#360

screenshot at 2018-07-13 17 06 22

I'd expect this to say "@chris48s set a reminder for Oct 31st 2018".

Also, side issue: Is there a way to unset/reschedule this now that this has happened?

@bkeepers
Copy link
Contributor

bkeepers commented Jul 13, 2018

Thanks for the report. I just tried to create a test case for https://github.com/bkeepers/parse-reminder and it seems to parse your example fine.

I'll test it out here:

/remind me "check the chicken - is it done?" on 31st Oct at 10:30am

GitHub
parse-reminder - a node module to parse natural language reminders into who, what, and when

@reminders reminders bot added the reminder label Jul 13, 2018
@reminders
Copy link

reminders bot commented Jul 13, 2018

@bkeepers set a reminder for Oct 31st 2018

@bkeepers
Copy link
Contributor

bkeepers commented Jul 13, 2018

@chris48s Very odd. I'm not able to duplicate it. I just pushed a test case to https://github.com/bkeepers/parse-reminder/compare/test-case-31st-oct and the test is passing.

If you can come up with a test case that reliably fails, I'd be happy to investigate further.

Also, side issue: Is there a way to unset/reschedule this now that this has happened?

2 options:

  1. Just set a new reminder, which will clear the old one.
  2. Remove the reminder label
GitHub
parse-reminder - a node module to parse natural language reminders into who, what, and when

@chris48s
Copy link
Contributor Author

I see. In the case I've posted, it looks like the fact that the reminder text contains the string "Oct" confuses things.
If I change that test case to:

'remind me "check Oct BoundaryLine release - does it have GSS codes for CEDs?" on 31st Oct at 10:30am': {
  who: 'me', when: new Date(2017, 9, 31, 10, 30, 0, 0), what: '"check Oct BoundaryLine release - does it have GSS codes for CEDs?"'
}

.. it becomes easy to see what has happened:

{
-  "what": "\"check  BoundaryLine release - does it have GSS codes for CEDs?\""
-  "when": [Date: 2017-10-01T09:00:00.000Z]
+  "what": "\"check Oct BoundaryLine release - does it have GSS codes for CEDs?\""
+  "when": [Date: 2017-10-31T10:30:00.000Z]
   "who": "me"
}

I accept this is an edge case because the string is somewhat ambiguous, but it is possible to construct some more unexpected examples. For example the following test case:

'remind me test prod again after next deploy - this may be fixed on 31st Oct at 10:30am': {
  who: 'me', when: new Date(2017, 9, 31, 10, 30, 0, 0), what: 'test prod again after next deploy - this may be fixed'
}

parses the word "may" as the date, rather than the string "31st Oct", which I think would be unexpected behaviour for most users:

{
-  "what": "test prod again after next deploy - this  be fixed"
-  "when": [Date: 2017-05-01T09:00:00.000Z]
+  "what": "test prod again after next deploy - this may be fixed"
+  "when": [Date: 2017-10-31T10:30:00.000Z]
   "who": "me"
}

This is definitely a much more subtle bug than I first thought I was reporting, but it could be a useful improvement to try and address it. Perhaps if the input string contains >1 strings which could potentially be parsed as a timestamp, the one which describes the most specific timestamp should be used, rather than the first one in the string?

Another possible weighting factor could be proximity to the word "at" or "on". If there's >1 strings which could be parsed as a date and one of them comes after the word "on", that's probably the one we want.

@stale
Copy link

stale bot commented Oct 11, 2018

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Oct 11, 2018
@reminders reminders bot removed the reminder label Oct 31, 2018
@reminders
Copy link

reminders bot commented Oct 31, 2018

👋 @bkeepers, "check the chicken - is it done?"

@stale stale bot closed this as completed Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants