chrono icon indicating copy to clipboard operation
chrono copied to clipboard

`DateTime::years_since` is tricky during ambiguous local time

Open pitdicker opened this issue 1 year ago • 5 comments
trafficstars

Suppose the clock is turned back on say 2024-03-31 from 3:00 to 2:00 due to DST. We want to compare the current time with 2023-03-31 at 2:30:

let now = Local::now();
let base = NaiveDate::from_ymd_opt(2023, 3, 31).unwrap().and_hms_opt(2, 30, 0).unwrap();
let years_since = now.years_since(base);

The current behavior is that on 2024-03-31 it will return 0 years until 2:30. From the first 2:30 until 3:00 it will return 1 year as result. From the second 2:00 until 2:30 it will return 0 years. And after that 1 year again.

I am not sure what the 'right' result should be, but going back and forth seems not to be it.

pitdicker avatar Jan 31 '24 21:01 pitdicker

We can get a sensible result if we create an intermediate value with the date of now and the time of base, picking the earliest datetime if ambiguous. Then we compare the times of the intermediate value and now. (This only has to happen if the month and day are the same.)

pitdicker avatar Jan 31 '24 21:01 pitdicker

I have questions:

  • Why does this behavior happen? Because a DST transition happens on that day, and we pick one of the sides?
  • Do we use years_since() in other calculations inside chrono?
  • If so, how, and what would the correct expected results be for those?

djc avatar Feb 01 '24 08:02 djc

I was just reading the implementation of DateTime::years_since and wondering how it dealt with differences in timezones. Does it work in local time or UTC? and if in local time, how does it behave if the two dates have different timezones?

Turns out it works in local time (sensible). The method signature requires the timezone to be the same. That only leaves the case of the offset from UTC changing within the same timezone as a potentially tricky case.

  • Why does this behavior happen? Because a DST transition happens on that day, and we pick one of the sides?

Because of a DST transition on that day, and we don't pick any side but naively use the local time.

  • Do we use years_since() in other calculations inside chrono?

No. It is used nowhere, we don't even seem to have tests. Actually I was in the process of deprecating it but couldn't come up with a usable alternative.

  • If so, how, and what would the correct expected results be for those?

I suppose from the moment month, day and time are equal it counts as a full year that has passed. If the clock turns backwards after that it should keep counting that full year.

Should we care about this issue? I just consider this a minor bug that is fun to fix (I also know more normal ways to have fun :smile:). Maybe even a good first issue with some mentoring. More importantly the solution informs how to do correct calculations with a CalendarDuration on DateTimes.

pitdicker avatar Feb 02 '24 15:02 pitdicker

It's probably worth fixing but also not very high priority? Could do something like comparing the months and if the difference is larger than 1 return, same for months and days, then convert both to Utc and compare that?

djc avatar Feb 07 '24 14:02 djc

I can probably write the fix described in https://github.com/chronotope/chrono/issues/1398#issuecomment-1920023008 and a test in two hours.

The reason to open this issue is that I should get in the habit of making notes such as this public instead of keeping a single line somewhere on my PC with 'TODO'. It seemed low priority enough for me to not work on directly and discuss in a PR :rofl:.

pitdicker avatar Feb 07 '24 14:02 pitdicker