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

Original Chrono maintainer's comments on COMPARE.md. #63

Closed
lifthrasiir opened this issue Jul 31, 2024 · 5 comments
Closed

Original Chrono maintainer's comments on COMPARE.md. #63

lifthrasiir opened this issue Jul 31, 2024 · 5 comments
Labels
question Further information is requested

Comments

@lifthrasiir
Copy link

As the founder and original designer of Chrono, thank you so much for your COMPARE.md document; it was something I really wanted to write it down and publish but never finished. I'll give some more context for individual items listed in the Chrono section, with an important caveat that this only reflects the original design, not the current design that has gone through multiple other maintainers and contributors.

Time zone database integration

I never got to this point by my own, but the tzfile crate is closer to my original intent: tz-generic types were designed to support this exact use case. Chrono-tz was more of a stopgap, an external contribution which I accepted because Chrono never had a proper tzdb integration implemented before that anyway.

I firmly believe that an application should never embed an up-to-date copy of tzdb at all, because it implies another maintenance burden to application developers and users. I even concluded that you can do that even on Windows, as long as you don't try to work with older dates. (Windows API exposes the most recent time zone offset change to my knowledge.) My belief largely comes from Python's experience, specifically where pytz provided a copy of tzdb and didn't work well in my opinion. jiff-tzdb-platform is a great direction in this ideal, though I also want to see that jiff-tzdb is eventually made redundant.

Jiff losslessly roundtrips time zone aware datetimes

Chrono predates a stable release of Serde and serialization and deserialization support was frankly an afterthought. I agree that RFC 9557 is a way to go, and Jiff surely benefits from this hindsight. Good job.

Jiff provides support for zone aware calendar arithmetic

I stress that this is a personal opinion, but I don't value zone-aware calendar arithmetic that much. In fact there isn't a single Zone-Aware Calendar Arithmetic method there, exact details have to be evaluated and specified for each concrete use case. Today, as of this writing, is 2024-07-31 in my time zone. What is 2024-07-31 plus 2 months? It depends on exact methods, and each method has its own justification and use cases:

  • No answer, if the parsed date is literally taken and manipulated before parsed back.
  • 2024-08-30, if we truncate any excess date components.
  • 2024-09-01, if we allow wrapping around.
  • 2024-10-31, if we absolutely have to keep the day of month.

Chrono now has some built-in arithmetic support for its date and time types, but I never added them myself with an intent that other dedicated crates fill the gap for each use case. I regret that I never expressed this intent publicly enough.

Jiff losslessly roundtrips durations

For the same reason as above, Duration was never designed to fully support ISO 8601 durations because then Chrono had to implement the full calendar arithmetic which is already ambiguous enough without time zone. Also note that Chrono's Duration predates std::time::Duration, which eventually posed a significant problem when time 0.1 had to be removed.

Jiff supports dealing with gaps in civil time

Exposing more information about a non-continuous calendar date and time is always good for diagnostics. Back in my time, Chrono was struggling to resolve edge case ambiguities and I never had time to look back the exact information exposed on ambiguity; I agree on your analysis in this regard.

Jiff supports rounding durations; Jiff supports zone-aware rounding of durations; Jiff supports re-balancing durations; Jiff supports zone-aware re-balancing of durations

I can't say much about duration rounding and balancing which came much later than my own maintainership, but I can personally say that it is a neat feature to have and Chrono's version could have been made neater with a conscious design. This is not to say that I find Jiff's API great though, because I'm yet to fully evaluate its use cases.

Jiff supports getting the nth weekday from the current date

I agree that this discrepancy in Chrono's API is not ideal (and can be fixed with a will).

Jiff supports detecting time zone offset conflicts

Same comments about RFC 9557 apply here, but Chrono does have a system to detect any other conflicts in parsed date and time strings. Any support for RFC 9557 should improve this item as well.

Jiff supports adding durations with calendar units

I don't think this is a good idea. Chrono was designed to be reasonably fast for average cases, but some use cases (e.g. logging) demand the best performance and it would be a disservice for Chrono to ignore such use cases. Chrono's base types are therefore made lightweight, and all heavy tz-aware stuffs are made pluggable via generic types. If I eventually got to the point that general ISO 8601 durations are needed, I would have made an additional type that encompasses a simpler Duration.

Chrono is faster than Jiff in some cases

I'm surprised that Chrono still remains reasonably fast! Some parts of Chrono (in particular, combined date-weekday calculations) were guided with a wrong assumption which I subsequently discovered and learned in a hard way, so I find it surprising that Chrono does perform in spite of these weaknesses.


As a concrete issue, I would be glad to hear about your opinions on my comments. You are also more than welcomed to amend COMPARE.md with these comments, as long as they are presented without a loss of context. Jiff seems like a good opportunity to upgrade the standard for Rust date and time handling---which I couldn't have fully done by my own---and collecting multiple viewpoints should be a worthy step forward.

@BurntSushi
Copy link
Owner

BurntSushi commented Jul 31, 2024

@lifthrasiir Hiya! Thanks for commenting! I'll do my best to respond to your points. :-)

I firmly believe that an application should never embed an up-to-date copy of tzdb at all, because it implies another maintenance burden to application developers and users. I even concluded that you can do that even on Windows, as long as you don't try to work with older dates. (Windows API exposes the most recent time zone offset change to my knowledge.) My belief largely comes from Python's experience, specifically where pytz provided a copy of tzdb and didn't work well in my opinion. jiff-tzdb-platform is a great direction in this ideal, though I also want to see that jiff-tzdb is eventually made redundant.

Yeah the Windows situation is unfortunate, but AFAIK it does not support historical time zones. The other major problem with just using the Windows API is that RFC 9557 support becomes challenging. Namely, RFC 9557 really wants a IANA TZ identifier. So if you just limit yourself to what the Windows API does without using tzdb, it's pretty hard to do lossless serialization.

Windows isn't the only one. Generally speaking, Unix is the only platform where one can reasonably expect (and of course it isn't guaranteed) to find a system tzdb. Other platforms, like WASM for example, don't have a tzdb either.

I don't think jiff-tzdb will be redundant any time soon unfortunately. It would require every platform to provide some way to access a "system" copy of tzdb. I don't think there's any movement in the Windows world to do this. And I don't think there's any movement in web browsers (for WASM) to do this either. Nor for WASI.

But yeah, I agree, prefer not bundling whenever possible. Which is what Jiff attempts to do.

Today, as of this writing, is 2024-07-31 in my time zone. What is 2024-07-31 plus 2 months?

I think this and time zone safe arithmetic are largely separable. This particular problem, for example, is present even if you remove "in my time zone." The calendar math alone is problematic and there are indeed multiple possible choices for this. I chose to go with what Temporal does here. There has been lots of discussion on this point:

And adjacent behaviors:

The TL;DR is that Temporal supports specifying whether something like "2024-07-31 + 2 months" should either "constrain" (the default) or reject:

>> dt = Temporal.PlainDateTime.from('2024-07-31T15:30');
Object { … }
>> dt.add({months: 2}).toString()
"2024-09-30T15:30:00"
>> dt.add({months: 2}, {overflow: 'reject'}).toString()
Uncaught RangeError: value out of range: 1 <= 31 <= 30

Jiff currently always constrains inputs by default, but my intent was to at least match Temporal here and provide a "reject" mode as well.

If you read the issues above, Temporal also used to support a "balance" mode that would return 2024-10-01 here, but it got removed because 1) they lacked use cases and 2) had some unintuitive behavior in some cases.

We probably have a philosophical difference here. I think that even though there isn't one obviously correct answer, it is possible to implement this sort of addition with a consistent semantic that is likely good enough for most use cases. And crucially, it should be possible for callers to implement their own semantic should they want it. (Which I believe is the case. Given Jiff's current API, I believe each of the cases you've shown are reasonably straight-forward to do.)

As far as zone aware, that's the difference between 2024-03-09T15:00-05[America/New_York] + 1 day resulting in either 2024-03-10T16:00-04[America/New_York] and 2024-03-10T15:00-04[America/New_York]:

use jiff::{ToSpan, Zoned};

fn main() -> anyhow::Result<()> {
    let zdt: Zoned = "2024-03-09T15:00-05[America/New_York]".parse()?;
    println!("{}", zdt.checked_add(1.day())?);
    println!("{}", zdt.checked_add(24.hours())?);

    Ok(())
}

Has this output:

$ cargo -q r
2024-03-10T15:00:00-04:00[America/New_York]
2024-03-10T16:00:00-04:00[America/New_York]

I feel strongly that this is desirable behavior. Chrono supports this too, but you need to use chrono::Days.

For the same reason as above, Duration was never designed to fully support ISO 8601 durations because then Chrono had to implement the full calendar arithmetic which is already ambiguous enough without time zone.

[.. snip ..]
I don't think this is a good idea. Chrono was designed to be reasonably fast for average cases, but some use cases (e.g. logging) demand the best performance and it would be a disservice for Chrono to ignore such use cases. Chrono's base types are therefore made lightweight, and all heavy tz-aware stuffs are made pluggable via generic types. If I eventually got to the point that general ISO 8601 durations are needed, I would have made an additional type that encompasses a simpler Duration.

Out of curiosity, have you taken a look at the Temporal TC39 proposal? They've spent person years on this. While Jiff's tzdb integration is perhaps one of the bigger differences with Chrono, I actually think that Jiff's Span type (a hybrid duration) is the "crown jewel." AFAIK, the Temporal project invented it. But because it's new, I think folks are having a hard time pattern matching it into their existing experience. I need to figure out how to highlight it better I think.

In any case, I talk a lot more about this here (including the perf topic): #21 --- While perf is one aspect to consider here, there's also the integration point with absolute-style durations like std::time::Duration. My plan as of now is to add a SignedDuration type to Jiff and add support for it to all the datetime types. This will mean you'll be able to do arithmetic without the Span type.

Same comments about RFC 9557 apply here, but Chrono does have a system to detect any other conflicts in parsed date and time strings. Any support for RFC 9557 should improve this item as well.

Can you say more about this? I don't think conflicts can be detected in a datetime string unless you support parsing the TZ identifier. Otherwise, when you serialize a datetime with Chrono, it will just include the offset. So when you go to deserialize it later, there is no way to check whether it's "correct" or not. You would have to store the TZ identifier out-of-band somewhere.

As a concrete issue, I would be glad to hear about your opinions on my comments. You are also more than welcomed to amend COMPARE.md with these comments, as long as they are presented without a loss of context. Jiff seems like a good opportunity to upgrade the standard for Rust date and time handling---which I couldn't have fully done by my own---and collecting multiple viewpoints should be a worthy step forward.

Thanks! So, COMPARE.md is meant to be the "facts of comparison." I tried hard not to express opinions in COMPARE.md. I think most of your commentary here is more contextual and opinion based (which is totally cool here on the tracker, I welcome it!) as opposed to correcting factual inaccuracies. (Please correct me if you think that's a wrong takeaway.) There is DESIGN.md, but that's more about Jiff's rationale and not Chrono's. However, I could link this issue from those documents to at least provide a gateway to an alternative perspective.

@BurntSushi BurntSushi added the question Further information is requested label Aug 3, 2024
@BurntSushi
Copy link
Owner

I've added a link to this issue in the _documentation::design document: f8cb134

@autarch
Copy link

autarch commented Nov 8, 2024

I'm not sure what you mean by a "hybrid duration", but if you mean a duration type that stores multiple units, that definitely wan't invented by Temporal. The Perl module Date-Ical, includes a Date::Ical::Duration type in the linked 1.61 release from December, 2001.

Python's stdlib has a similar timedelta type that goes back to at least 2.6.

@BurntSushi
Copy link
Owner

BurntSushi commented Nov 8, 2024

timedelta is definitely not storing every unit separately:

>>> from datetime import timedelta
>>> td1 = timedelta(hours=2)
>>> td2 = timedelta(minutes=120)
>>> td1
datetime.timedelta(seconds=7200)
>>> td2
datetime.timedelta(seconds=7200)

And from your Perl docs:

Internally, we store 3 data values: a number of days, a number of seconds (anything shorter than a day), and a sign (1 or -1). We are assuming that a day is 24 hours for purposes of this module; yes, we know that's not completely accurate because of daylight-savings-time switchovers, but it's mostly correct. Suggestions are welcome.

In contrast, a Span stores every unit separately.

See #17 where someone else also thought Span was not novel and was done before by PostgreSQL.

See tc39/proposal-temporal#2915 (comment) where I asked the Temporal folks for prior art on their Duration type.

And see tc39/proposal-temporal#2535 (comment) where they did an investigation. (Date::Ical::Duration is not included as part of their investigation.)

From the Temporal folks, this might help (emphasis mine):

For end-user-facing formatting, "36 hours" is distinct from "1 day and 12 hours" because some use cases require the former format and some the latter. This is not unique to durations larger than a day, e.g. "90 minutes" vs. "1 hour and 30 minutes". Temporal's Duration objects are intentionally designed to be the input shape to a localized duration formatting API like DurationFormat that allows the formatting API to be "dumb" meaning it simply prints what you give it, instead of requiring the formatting API to perform complex DST calculations. If we split duration types, then DurationFormat would need to be responsible for combining them, which we thought was a bad idea.

@autarch
Copy link

autarch commented Nov 8, 2024

Ah, I see what you mean. Yes, I don't think I've seen a Span type like this that stores units that can be converted between each other separate (like minutes and hours).

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

No branches or pull requests

3 participants