haskell / time

A time library
http://hackage.haskell.org/package/time
Other
118 stars 78 forks source link

Confusing diffGregorianDuration* output #230

Closed LiamGoodacre closed 1 year ago

LiamGoodacre commented 1 year ago

Using time ==1.11.1.1.

Here are some differences calculated using diffGregorianDurationClip & diffGregorianDurationRollOver:

2001-02-28 – 2000-01-30
  clip: (12,29)
  roll: (12,29)
2001-03-01 – 2000-01-30
  clip: (13,1)
  roll: (13,-1)
2001-03-02 – 2000-01-30
  clip: (13,2)
  roll: (13,0)
2001-03-03 – 2000-01-30
  clip: (13,3)
  roll: (13,1)
2001-03-04 – 2000-01-30
  clip: (13,4)
  roll: (13,2)
(Generated using this code) ```haskell module Main where import Data.Time main :: IO () main = do let dump t v = putStrLn $ " " <> t <> ": " <> show (cdMonths v, cdDays v) let check e s = do putStrLn $ show e <> " – " <> show s dump "clip" (diffGregorianDurationClip e s) dump "roll" (diffGregorianDurationRollOver e s) mapM_ (\i -> check (addDays i (fromGregorian 2001 2 28)) (fromGregorian 2000 1 30) ) [0..4] -- $> main ```

The docs for diffGregorianDurationRollOver suggest that for positive durations, it should give the same output as diffGregorianDurationClip.

Have I discovered a bug? Am I just confused? (maybe both?)

AshleyYakeley commented 1 year ago

Hmm, these should satisfy:

addGregorianDurationClip (diffGregorianDurationClip a b) b = a
addGregorianDurationRollOver (diffGregorianDurationRollOver a b) b = a

2000-01-30 + 13 months gives "2001-02-30", which is then clipped to 2001-02-28, or rolled over to 2001-03-02.

AshleyYakeley commented 1 year ago

I think these results satisfy that? However, the -1 is surprising. This would also work:

2001-02-28 – 2000-01-30
  clip: (13,0)
  roll: (12,29)
2001-03-01 – 2000-01-30
  clip: (13,1)
  roll: (12,30)
2001-03-02 – 2000-01-30
  clip: (13,2)
  roll: (13,0)
2001-03-03 – 2000-01-30
  clip: (13,3)
  roll: (13,1)
2001-03-04 – 2000-01-30
  clip: (13,4)
  roll: (13,2)

Of course, now this means the number of months may be different.

LiamGoodacre commented 1 year ago

Thanks for responding @AshleyYakeley! What you've said has made sense to me. Your example output eliminating the -1 feels a lot nicer to me; including in the sense that it looks like there aren't any "jumps": e.g consecutively, the clip currently goes (12,28) to (12,29) to (13,1) (where is (13,0)); instead it'd be (12,28) to (13,0) to (13,1).

Regarding the docs saying they'll be the same for positive durations; have I also misunderstood what a positive duration means here?

AshleyYakeley commented 1 year ago

No, I think that's wrong and should be deleted.

LiamGoodacre commented 1 year ago

Though it shouldn't necessarily have any bearing on what you decide to do, for comparison here's what the luxon JavaScript library produces:

2001-02-27 – 2000-01-30
  { years: 1, months: 0, days: 28 }
2001-02-28 – 2000-01-30
  { years: 1, months: 1, days: 0 }
2001-03-01 – 2000-01-30
  { years: 1, months: 1, days: 1 }

Using:

Interval.fromDateTimes(start, end)
  .toDuration(
    ['years', 'months', 'days'],
    {conversionAccuracy: 'longterm'}
  ).toObject()
AshleyYakeley commented 1 year ago

OK, done.