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

Relative times containing years fail when computed from a leap day #155

Closed
schoen opened this issue Feb 29, 2016 · 16 comments
Closed

Relative times containing years fail when computed from a leap day #155

schoen opened this issue Feb 29, 2016 · 16 comments

Comments

@schoen
Copy link

@schoen schoen commented Feb 29, 2016

When it is February 29 (like today!), parsedatetime.Calendar().parse("1 year") raises

ValueError: day is out of range for month

(It's trying to find February 29 in the following non-leap year, i.e., February 29, 2017.)

I think this is unexpected behavior because people would expect to be able to specify relative time intervals in terms of years on any day. (For example, we had written code that always computes parse("1 year") relative to the present day, and it crashes when run today!)

@idpaterson
Copy link
Collaborator

@idpaterson idpaterson commented Feb 29, 2016

I'm getting Feb 28, 2017 – are you using a specific version, or master?

@schoen
Copy link
Author

@schoen schoen commented Feb 29, 2016

@idpaterson, 1.5 from PyPi.

@bear
Copy link
Owner

@bear bear commented Feb 29, 2016

Last week I was thinking we should push a new version...

@mistercrunch
Copy link

@mistercrunch mistercrunch commented Feb 29, 2016

+1 just hit this problem today

@mistercrunch
Copy link

@mistercrunch mistercrunch commented Feb 29, 2016

I just confirmed that this is fixed in master who can push a version?!

@idpaterson
Copy link
Collaborator

@idpaterson idpaterson commented Feb 29, 2016

@mistercrunch there are a lot of changes in master, so that's probably not going to happen today but I'm glad to hear that it's on your mind, @bear!

@mistercrunch
Copy link

@mistercrunch mistercrunch commented Feb 29, 2016

Maybe cherry pick the commit that fixes this and push a hotfix to pypi?

@mistercrunch
Copy link

@mistercrunch mistercrunch commented Feb 29, 2016

It master stable? I was going to install it on my production boxes.

@idpaterson
Copy link
Collaborator

@idpaterson idpaterson commented Feb 29, 2016

It must have been an incidental fix; there are no commit messages, comments, or test cases specifically addressing the leap day. A pull request against version 1.5 may be appropriate if anyone affected has time to find and patch the issue.

I have been using parsedatetime master in production on an Alfred workflow since Feb 2015 with no problems (primarily using parse and nlp against today-relative user input dates) currently up-to-date with the latest master. The test coverage is excellent though datetime-related edge cases are inherently difficult to identify and test.

@bear
Copy link
Owner

@bear bear commented Feb 29, 2016

I can push a version now if that is needed.

@bear
Copy link
Owner

@bear bear commented Feb 29, 2016

v2.0 has been released

@bear bear mentioned this issue Feb 29, 2016
@mistercrunch
Copy link

@mistercrunch mistercrunch commented Feb 29, 2016

@bear saves the day. See you in 4 years!

@bear
Copy link
Owner

@bear bear commented Feb 29, 2016

👍 marking issue as closed!

@bear bear closed this Feb 29, 2016
@schoen
Copy link
Author

@schoen schoen commented Feb 29, 2016

@bear, thanks for the new release. I agree it fixes this problem, although please also see #156 because our project hopes to support Python 2.6 (which has a problem with 2.0).

@bear
Copy link
Owner

@bear bear commented Feb 29, 2016

@schoen I will look at that now

@mistercrunch
Copy link

@mistercrunch mistercrunch commented Mar 1, 2016

@schoen , many libs are dropping support for 2.6, you should consider moving up to 2.7 or go straight to 3.4

@idpaterson idpaterson mentioned this issue Sep 23, 2016
25 of 31 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants