search

metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 204 forks source link

Add :time/period schema to experimental.time #957

Closed dvingo closed 8 months ago

dvingo commented 9 months ago

Adds :time/period to the experimental time schemas.

ikitommi commented 9 months ago

Thanks for the PR!

I looked quickly at ISO_8601 and it describes also time-based durations, e.g. syntax of PnYnMnDTnHnMnS.

For example, "P3Y6M4DT12H30M5S" represents a duration of "three years, six months, four days, twelve hours, thirty minutes, and five seconds".

TC39, it's called duration and supports also both dates and times.

In js-joda & java-time, Period and Duration are separate things:

When you write code to specify an amount of time, use the class or method that best meets your needs: the Duration class, Period class, or the ChronoUnit.between method. A Duration measures an amount of time using time-based values (seconds, nanoseconds). A Period uses date-based values (years, months, days).

options:

  1. go just with java-time/js-joda Period, add later separate Duration
  2. go just with java-time/js-joda Period, add later Duration that is the full ISO-thing
  3. go now with :time/duration which supports both Period & Duration

compliancy-wise I would like to see 3, but might be awkward with java-time & js-joda apis?

dvingo commented 9 months ago

Thanks for the review and feedback!

I wasn't aware about ISO_8601 and TC39 combining the two together, I have only been using the java.time (and js-joda) versions in my applications and so that's where the Period/Duration distinction is coming from.

Incidentally, I have felt the need for combining the two together for a while and made my own abstraction called Offset here: https://github.com/dvingo/tick-util/blob/master/src/main/space/matterandvoid/tick_utils/offset.cljc it serializes to a string differently than described by ISO_8601, but that could easily be changed.

I was under the impression that the malli :time/* schemas are intended to follow the semantics of java.time, but it seems like the desire is to have something that matches the standards instead.

Perhaps the Offset idea could be adapted to implement a malli :time/iso-duration (or different name) to avoid conflict with the java.time semantics of the duration name?

ikitommi commented 9 months ago

The original idea was to make something "final" but I don't think it's possible here. e.g. java.time is de facto and I guess TC39 will be too. But they have different concepts.

Current time is modelled based on java.time & js-joda, so I think this PR makes a lot of sense - and the PR is well crafted.

Will get back to this soon.

ikitommi commented 8 months ago

yes, let's do this. Thanks!!