periodic-seq example and DST changes

[terjesb]

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]

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