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

Behaviour change when comparing Date and Time between previous versions and 3.3.0 #86

Closed
JonathanTron opened this issue Jan 23, 2023 · 8 comments

Comments

@JonathanTron
Copy link

Hi there,

I'm not sure this is the right place to put such issue, but I found an issue after updating from 3.0.1 to 3.3.3.

I tracked the change in behaviour to start appearing between versions 3.2.2 and 3.3.0, here's and example of the change:

with version 3.2.2

require "time" 
require "date"

Time.parse("1000-01-01T00:00:00Z")
# date v3.2.2 => "1000-01-01T00:00:00.000Z"

Time.parse("1000-01-01T00:00:00Z").to_date
# date v3.2.2 => Wed, 27 Dec 0999

Date.parse("1000-01-01") == Time.parse("1000-01-01T00:00:00Z")
# date v3.2.2 => true

Time.parse("1000-01-01T00:00:00Z") == Date.parse("1000-01-01")
# date v3.2.2 => true

with version 3.3.0

require "time" 
require "date"

Time.parse("1000-01-01T00:00:00Z")
# date v3.3.3 => "1000-01-01T00:00:00.000Z"

Time.parse("1000-01-01T00:00:00Z").to_date
# date v3.3.0 => Wed, 27 Dec 0999

Date.parse("1000-01-01") == Time.parse("1000-01-01T00:00:00Z")
# date v3.3.3 => false

Time.parse("1000-01-01T00:00:00Z") == Date.parse("1000-01-01")
# date v3.3.3 => false

As you can see I'm working with date in the Julian calendar and comparing them used to work before 3.3.0.

Now I'm not sure if and what is the bug there, as this was maybe already an issue:

Time.parse("1000-01-01T00:00:00Z").to_date
# => Wed, 27 Dec 0999

But in the end, this change of behaviour surfaced and I wanted to be sure it's reported.

@zverok
Copy link
Contributor

zverok commented Jan 23, 2023

I am not sure about the overall topic, but it deserves noting that Time and Date aren't comparable by default, as far as I remember, and never were. E.g. in pure Ruby, without additions like ActiveSupport, you should expect this:

require 'date'
require 'time'

d = Date.new(2023, 1, 23)
t = Time.utc(2023, 1, 23)
d == t
#=> false
d <=> t
#=> nil

...so, whatever produced

Date.parse("1000-01-01") == Time.parse("1000-01-01T00:00:00Z")
#=> true

...was NOT Ruby's core Date or Time. It might've been ActiveSupport:

require 'active_support/all'
d == t
#=> true
d <=> t
#=> 0
Date.parse("1000-01-01") == Time.parse("1000-01-01T00:00:00Z")
#=> true for me in Ruby 2.7
#=> false for me in Ruby 3.2

ActiveSupport uses Time.to_datetime coercion underneath, and it indeed have changed:

Time.parse("1000-01-01T00:00:00Z").to_datetime
#=> Mon, 01 Jan 1000 00:00:00 +0000 on 2.7
#=> Wed, 27 Dec 0999 00:00:00 +0000 on 3.2

...which is now consistent with to_date (which is weird, but was weird the same way in Ruby 2.7 and Ruby 3.2).

I don't feel qualified or have enough time right now to analyze this behavior.

@zverok
Copy link
Contributor

zverok commented Jan 23, 2023

...but it seems to be related to this PR: #73

@nobu
Copy link
Member

nobu commented Jan 24, 2023

Time has used the proleptic Gregorian calendar from the beginning (more precisely, it has never considered the Julian calendar), and Date uses the Italian reform date by default.
That means these two dates are actually different days.

@JonathanTron
Copy link
Author

JonathanTron commented Jan 24, 2023

@zverok Thanks for checking this out. I intentionaly reduced my test case to ruby core/stdlib only, no rails lib in there. Also my issue is a difference even with the same Ruby version, but different date gem versions.

@nobu Thanks for your answer, I get the difference but then isn't it an inconsistency between Time and Date that should be sorted out?

Let's say I have a db (or json, csv, etc.) with:

date time
1000-01-01 1000-01-01T00:00:00Z

How am I supposed to parse them so they are considered to be on the same day?

@zverok
Copy link
Contributor

zverok commented Jan 24, 2023

@JonathanTron

Thanks for checking this out. I intentionaly reduced my test case to ruby core/stdlib only, no rails lib in there.

If it would be so, == between Date and Time would never return true. Unless I am missing something important (but I checked thoroughly before writing).

isn't it an inconsistency between Time and Date that should be sorted out?

Basically, date standard library is considered by core devs as a "scientific" library (while the core Time class is more simple/pragmatic).
Thus, core Time doesn't support non-Gregorian calendars and shouldn't be used for dates where it is inappropriate.
This situation is not ideal and not obvious, especially coming from Rails which liberally use Date for all dates representation, but it is what it is.

date standard library, on the other hand, has a calendar-aware DateTime class. So,

How am I supposed to parse them so they are considered to be on the same day?

Date.parse('1000-01-01') == DateTime.parse('1000-01-01T00:00:00Z')
#=> true

@JonathanTron
Copy link
Author

@zverok

Thanks for checking this out. I intentionaly reduced my test case to ruby core/stdlib only, no rails lib in there.
If it would be so, == between Date and Time would never return true. Unless I am missing something important (but I checked thoroughly before writing).

Indeed, I checked with a clean slate docker ruby:

> docker run -it --rm ruby:3.0-slim gem list | grep date
date (default: 3.1.3)
docker run -it --rm ruby:3.0-slim irb
require "date"
d = Date.new(1000, 1, 1)
# => #<Date: 1000-01-01 ((2086308j,0s,0n),+0s,2299161j)>

t = Time.new(1000, 1, 1, 0, 0, 0)
# => 1000-01-01 00:00:00 +0000

t == d
# => false

d == t
=> false

How am I supposed to parse them so they are considered to be on the same day?

Date.parse('1000-01-01') == DateTime.parse('1000-01-01T00:00:00Z')
#=> true

Ok, for it to work, everything has to use only Date and DateTime.

One last example which is then also confusing:

require "date"
d = Date.new(1000, 1, 1)
# => #<Date: 1000-01-01 ((2086308j,0s,0n),+0s,2299161j)>

t = Time.new(1000, 1, 1, 0, 0, 0)
# => 1000-01-01 00:00:00 +0000

dt = t.to_datetime

dt == d
# => true

d == dt
=> true

So going from Time to Date is not the same as going from Time to DateTime.

@zverok
Copy link
Contributor

zverok commented Jan 24, 2023

Ok, for it to work, everything has to use only Date and DateTime.

Yes, date standard library promises and provides no direct compatibility (other than conversion) with Time core class. As I've said above, this situation is not ideal and not obvious, but it is what it is.

One last example which is then also confusing:

I am not sure what it demonstrates?.. For me on Ruby 3.2 it is

d = Date.new(1000, 1, 1)
# => #<Date: 1000-01-01 ((2086308j,0s,0n),+0s,2299161j)>

t = Time.new(1000, 1, 1, 0, 0, 0)
# => 1000-01-01 00:00:00 +0000

dt = t.to_datetime
# => Wed, 27 Dec 0999 00:00:00 +0202

dt == d
# => false

...which is consistent with parse-based examples above.

Before Ruby 3.2, it was:

dt = t.to_datetime
# => #<DateTime: 1000-01-01T00:00:00+02:02 ((2086307j,79076s,0n),+7324s,2299161j)> 

...which was a calendar-ignoring bug fixed by #73, the situation is no different for what we discussed about #parse

@JonathanTron
Copy link
Author

JonathanTron commented Jan 24, 2023

@zverok Indeed, my last example is something which worked also before but not after the fix from #73.

Thanks again for taking the time to dig and explain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants