ruby / date

A subclass of Object includes Comparable module for handling dates.
Other
73 stars 38 forks source link

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

Closed JonathanTron closed 1 year ago

JonathanTron commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

...but it seems to be related to this PR: https://github.com/ruby/date/pull/73

nobu commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 https://github.com/ruby/date/pull/73, the situation is no different for what we discussed about #parse

JonathanTron commented 1 year ago

@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.