olliemath / chronoutil

ChronoUtil module provides powerful extensions to rust's Chrono crate.
MIT License
22 stars 9 forks source link

ISO 8601 parser. Wanted feature? #13

Closed intarga closed 5 months ago

intarga commented 10 months ago

So, I wrote an ISO 8601 subset parser for my own purposes, and I was wondering it it's a wanted feature? Personally I've ended up needing it everywhere I use chronoutil.

Currently it doesn't support decimals or weeks, because I didn't need either, but I could add those and submit a PR if it's wanted.

olliemath commented 8 months ago

Hey, thanks for this and sorry it's taken so long to get back to you.

Serialisation / deserialisation for durations has come up a couple of times. My main argument for not including it is that chrono doesn't include anything for their own durations, but also it's not a great standard for durations. However I'm willing to include it if we can get a "good enough" implementation.

There are a few thorny issues to solve - namely:

  1. dealing with negative durations (I believe not part of the standard)
  2. excluding fractional durations (P0,5Y is valid iso8601, but we don't allow fractions of years or months due to ambiguity)
  3. decide on the relativeduration -> iso representation - should we map 13 months to P1Y1M or P13M? should this be configurable?
intarga commented 8 months ago

No worries! And thanks for getting back to me.

  1. Do you have a particular viewpoint on negative durations? Personally I would not be inclined to support them at all, since the standard doesn't seem to specify a format for them. I would just return an error on attempt to serialise a negative duration.
  2. I would put a warning in the docs and error on deserialisation.
  3. I think there should be one canonical representation, and it should be the simplest. IMO this is start with the largest increment, and set it to its max before going to the next. So in the case of your example, P1Y1M.

I'll also add a fourth thorny issue:

  1. What to do about weeks (the spec allows for things like P5W)? For deserialisation, I think it should be ok to just times by 7 and add this as days the the chrono duration. For serialisation I don't think we should use weeks at all.
olliemath commented 8 months ago
  1. a lot of prior art tends to allow negative duration serialisation/deserialisation (in that sense it's an extension to the standard - see e.g. momentjs https://momentjs.com/docs/#/durations/) - I'd be tempted to go the same route (perhaps strict serialisation could be an optional feature)
  2. nice, that's probably as good as we can get
  3. yes, one canonical representation is good. Since Duration and RelativeDuration I think store everything in the min increment, I think it will be easier on a first pass to implement P13M etc.
  4. good catch - yes multiplying by 7 works nicely
intarga commented 8 months ago

Ok, it seems like we have a clear path forward. I’m a little busy right now, but I’ll refine my implementation and file a PR when i have time

olliemath commented 8 months ago

Thanks! No rush :)