rluiten / elm-date-extra

Elm Date Extra library add/subtract/diff/format etc dates
http://package.elm-lang.org/packages/rluiten/elm-date-extra/latest
BSD 3-Clause "New" or "Revised" License
75 stars 36 forks source link

Problem using period with a timezone #49

Closed Fenntasy closed 7 years ago

Fenntasy commented 7 years ago

Hi, I'm the author of the article about time manipulation in elm and sorry for the delay of response, I had to rethink like my old self.

I make an Ellie App that still seems to reproduce what my problem was (but having changed DST, only one month is problematic at the moment).

The result of that code is this (it is for 2016):

Jan: 31
Feb: 29
Mar: 30
Apr: 30
May: 31
Jun: 30
Jul: 31
Aug: 31
Sep: 30
Oct: 31
Nov: 30
Dec: 31

Notice that March is displaying only 30 days.

To be fair, it could just be something wrong in my way of calculating the number of days. I was doing it that way to be consistent with my calculation of days without weekend, that I did thusly:

numberOfDaysBetweenNotCountingWeekend : Date -> Date -> Int
numberOfDaysBetweenNotCountingWeekend start end =
    Period.diff start end
        |> (\d -> (d.week * 5) + d.day)
        |> abs
        |> (+) 1

I was using elm-date-extra 8.1.2 at the time, but I tested on Ellie with the 9.0.1 which I assume is the most recent one.

Thanks for asking me to do this, I should have done it form the start :)

Fenntasy commented 7 years ago

PS: I forgot maybe something important, my timezone is Europe/Paris

rluiten commented 7 years ago

Thanks for the report, I believe the behavior you are seeing is part of the design of Period functions what you want for day light saving handling is the Duration functions.

Period is a fixed length of time. It is an elapsed time concept, which does not include the concept of Years Months or Daylight saving variations.

Duration is a length of time that may vary with calendar date and time. It can be used to modify a date.

I forked and modified your example to use Duration https://ellie-app.com/3KkfdF5yf9qa1/1 and it appears to be produce the right results. I did modify my notebook here to Paris time zone and saw the beahaviour you describe, the Duration variant didn't not show the behaviour.

I think the problem is that you are using Period not Duration.

Fenntasy commented 7 years ago

Thanks for the infos, I will remember it and sorry for the bad press when it was my fault.

Fenntasy commented 7 years ago

I added a mention that it was my fault, linking to this issue on the article, just so you know :)

rluiten commented 7 years ago

Its all good, I was just interested in what was up when I stumbled upon your article :).