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

Time#to_datetime incorrect for times before the date of calendar reform #4822

Closed
jeremyevans opened this issue Oct 18, 2017 · 4 comments
Closed
Labels

Comments

@jeremyevans
Copy link
Contributor

@jeremyevans jeremyevans commented Oct 18, 2017

Environment

jruby 9.1.13.0 (2.3.3) 2017-09-06 8e1c115 OpenJDK 64-Bit Server VM 25.144-b01 on 1.8.0_144-b01 +jit [OpenBSD-x86_64]
OpenBSD systemname.local 6.2 GENERIC.MP#140 amd64

Also tested on:

jruby 9.1.12.0 (2.3.3) 2017-06-15 33c6439 Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 +jit [mswin32-x86_64]
Windows 10 Professional

Expected Behavior

Same Results as CRuby for the following code:

require 'date'
# after date of calendar reform in 1582: correct
p Time.at(-12000000000).utc
p Time.at(-12000000000).utc.to_datetime 
# before date of calendar reform in 1582: incorrect
p Time.at(-12500000000).utc
p Time.at(-12500000000).utc.to_datetime 

CRuby Results:

1589-09-26 02:40:00 UTC
#<DateTime: 1589-09-26T02:40:00+00:00 ((2301699j,9600s,0n),+0s,2299161j)>
1573-11-22 01:46:40 UTC
#<DateTime: 1573-11-22T01:46:40+00:00 ((2295922j,6400s,0n),+0s,2299161j)>

Actual Behavior

JRuby Results:

1589-09-26 02:40:00 UTC
#<DateTime: 1589-09-26T02:40:00+00:00 ((2301699j,9600s,0n),+0s,2299161j)>
1573-11-22 01:46:40 UTC
#<DateTime: 1573-11-12T01:46:40+00:00 ((2295912j,6400s,0n),+0s,2299161j)>

Note the change from 1573-11-22 to 1573-11-12 in last line of output.

@headius
Copy link
Member

@headius headius commented Oct 30, 2017

I suspect this is because Joda Time, the time library we're using, defaults to ISO8601 time, while I believe UNIX time is Gregorian/Julian. I'm testing a patch that uses GJ, but I'm on the fence about fixing this.

@headius
Copy link
Member

@headius headius commented Oct 30, 2017

Hmm actually this logic is mostly contained in date.rb, which does already use the Gregorian/Julian chronology by default.

My patch to modify RubyTime in a similar way did not appear to help: https://gist.github.com/headius/6ee8683674d1c38a330b1a6c7bc7d5f6

@headius
Copy link
Member

@headius headius commented Oct 30, 2017

Ok interestingly enough, if I modify our date library to use ISO chronology, we match MRI.

[--dev] ~/projects/jruby $ jruby time_bug_4822.rb 
1589-09-26 02:40:00 UTC
ISOChronology[UTC]
#<DateTime: 1589-09-26T02:40:00+00:00 ((2301699j,9600s,0n),+0s,2299161j)>
1573-11-12 01:46:40 UTC
ISOChronology[UTC]
#<DateTime: 1573-11-22T01:46:40+00:00 ((2295912j,6400s,0n),+0s,2299161j)>

So I'm not sure who's right now.

@eregon
Copy link
Member

@eregon eregon commented Nov 3, 2017

Fixed by 8da4e66 on master, see #4810 for details.

@headius headius added this to the JRuby 9.1.14.0 milestone Nov 8, 2017
@headius headius closed this Nov 8, 2017
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
3 participants
You can’t perform that action at this time.