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

periodic-seq example and DST changes #54

Open
terjesb opened this issue Nov 2, 2022 · 1 comment
Open

periodic-seq example and DST changes #54

terjesb opened this issue Nov 2, 2022 · 1 comment
Assignees

Comments

@terjesb
Copy link

terjesb commented Nov 2, 2022

Thank you for this nice little library.

The README has an example of using periodic-seq for local times:

(chime/periodic-seq (-> (LocalTime/of 20 0 0)
                        (.adjustInto (ZonedDateTime/now (ZoneId/of "America/New_York")))
                        .toInstant)
                    (Period/ofDays 1))

I don't get this to work properly across DST changes. It seems to me that this example converts to instant before adding the period, which means that the period is always from UTC and not local time.

This shows the problem:

(binding [chime.core/*clock* (java.time.Clock/fixed (.toInstant #inst "2022-10-28T12:12:12Z") (java.time.ZoneId/of "UTC"))]
  (take 2
        (->> (chime.core/periodic-seq
              (-> (java.time.LocalTime/of 7 30)
                  (.adjustInto (-> (java.time.ZonedDateTime/now chime.core/*clock*)
                                   (.withZoneSameLocal (java.time.ZoneId/of "Europe/Oslo"))))
                  .toInstant)
              (java.time.Period/ofDays 1))
             (chime.core/without-past-times))))

Here we see that the result is the same UTC hour before before and after the recent DST change, which results in an incorrect local time for the second instant:

(#object[java.time.Instant 0x300c7735 "2022-10-29T05:30:00Z"]
 #object[java.time.Instant 0x54e5895a "2022-10-30T05:30:00Z"])

The problem seems to be that periodic-seq works with instants, but that the conversion to instant should instead be the final step. Not yet sure how to solve this in a nice way. A separate function for periodic-local-date-time-seq, and let the caller map to instants?

Or perhaps add an arity to periodic-seq for [LocalDateTime ZoneId duration-or-period] for calculating using LocalDateTimes, while still returning instants? (If so, maybe also include another arity for a final xform for doing complex schedules on the local date times, before returning the instants?)

In the mean time, here's a possible not-so-nice solution:

(binding [chime.core/*clock* (java.time.Clock/fixed (.toInstant #inst "2022-10-28T12:12:12Z") (java.time.ZoneId/of "UTC"))]
  (take 2
        (->> (map #(.toInstant (.atZone % (java.time.ZoneId/of "Europe/Oslo")))
                  (iterate
                   #(.plusDays % 1)
                   (-> (java.time.LocalTime/of 7 30)
                       (.adjustInto (java.time.LocalDateTime/now chime.core/*clock*)))))
             (chime.core/without-past-times))))

Here we see that the instants change as expected after DST change:

(#object[java.time.Instant 0x221b9091 "2022-10-29T05:30:00Z"]
 #object[java.time.Instant 0x49e3eb99 "2022-10-30T06:30:00Z"])
@jarohen jarohen self-assigned this Nov 9, 2022
@jarohen
Copy link
Owner

jarohen commented Jan 30, 2023

Hey @terjesb - I'm really sorry, this one slipped through my net 🤦

Absolutely agree - we prefer Instants when we actually come to scheduling the function, but if you're scheduling you may not want the same UTC time every day if you want it at 07:30 local. We could go for a separate arity, or maybe we could also accept java.time.Temporal as the first param (implemented by both Instant and ZDT), which should then return an object of the same type that the user passes in?

e.g., adapting your example

(->> (chime.core/periodic-seq (java.time.ZonedDateTime/parse "2022-10-29T07:30+02:00[Europe/Oslo]")
                              (java.time.Period/ofDays 1))
     (take 2))
;; => ("2022-10-29T07:30+02:00[Europe/Oslo]", "2022-10-30T07:30+01:00[Europe/Oslo]")

or, if we (map .toInstant)

(->> (chime.core/periodic-seq (java.time.ZonedDateTime/parse "2022-10-29T07:30+02:00[Europe/Oslo]")
                              (java.time.Period/ofDays 1))
     (map #(.toInstant ^java.time.ZonedDateTime %))
     (take 2))
;; => ("2022-10-29T05:30:00Z", "2022-10-30T06:30:00Z")

Does this match with what you'd expect?

Apologies again for the delay in getting back to you,

James

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

No branches or pull requests

2 participants