-
Notifications
You must be signed in to change notification settings - Fork 501
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
Add rounding for date parser #501
base: master
Are you sure you want to change the base?
Conversation
To avoid float precision mistake, e.g. 42988.99999999999 (11 September 2017) should be parsed same as 42989.0 to match with date time parser and excel behaviour
Found some strange behaviour of ruby Date object: d = Date.new(2017, 9, 11) + 15/24r
# d => #<Date: 2017-09-11 ((2458008j,54000s,0n),+0s,2299161j)>
# ^ fractional part, 54,000 seconds
# in rails it will be printed as "Mon, 11 Sep 2017"
# no time info
d.to_time # => 2017-09-11 00:00:00 +0800
d.to_datetime # => #<DateTime: 2017-09-11T00:00:00+00:00 ((2458008j,0s,0n),+0s,2299161j)>
d == Date.parse("Mon, 11 Sep 2017") # => false (difficult to understand reason)
# has time info
d.strftime("%F %T %z") # => "2017-09-11 15:00:00 +0000"
d.day_fraction # => (5/8) Imo it's difficult to understand that date may have fractional part, especially in rails What do you think? |
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.
Thank you for opening this PR. Can you add a relevant test case for this? Adding test case will ensure that this case is covered in the future as well.
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.
LGTM. Can you add a relevant test case to cover this scenario?
Sorry, I'm not familiar with your testing setup |
Could you give a hint on how to make tests? |
To avoid float precision mistake, e.g. 42988.99999999999 (11 September 2017) should be parsed same as 42989.0 to match with date time parser and excel behaviour
Fixes #500