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

Add jiff support #243

Merged
merged 28 commits into from
Oct 5, 2024
Merged

Add jiff support #243

merged 28 commits into from
Oct 5, 2024

Conversation

chmp
Copy link
Owner

@chmp chmp commented Oct 1, 2024

Implement #213

  • jiff::Date
    • normalize the format for negative dates: jiff expects 6 digits for negative years, when prefixed with a sign chrono allows an arbitrary number of digits (source]
    • check guess date support
  • jiff::Time
    • check guess date support
  • jiff::DateTime
  • jiff::Timestamp
  • jiff::Span
    • distinguish between durations that can be expressed as seconds (arrow: Duration) and calendar interval durations (arrow: Interval)
    • Note: implement interval support as part of Add support for Interval types #178
  • jiff::SignedDuration
  • jiff::Zoned
    • How to work around the different string formats for datetimes with timezones? (+00:00 for chrono, [UTC] for jiff)?
    • Probably, does not make sense. See discussion below
  • Allow to map Date to Date64 requires breaking changes, implement as part of [breaking] Fix Date64 semantics #240
  • Update changelog
  • Update docs (status.md)
  • Cleanup the impl
    • move tests that do not depend on arrow out side test_with_arrow tree

@chmp chmp marked this pull request as draft October 1, 2024 15:20
@chmp chmp mentioned this pull request Oct 1, 2024
@BurntSushi
Copy link

How to work around the different string formats for datetimes with timezones? (+00:00 for chrono, [UTC] for jiff)?

Chrono doesn't have anything like Zoned. The closest you can get is chrono::DateTime<T> where T is a time zone from chrono-tz or tzfile. But neither of those support Serde.

The [UTC] comes from RFC 9557. Chrono has no support for RFC 9557. The Jiff docs talk a bit about this.

If you want to serialize something like 2024-10-01T00:00:00-04:00 with Jiff, you can use a Timestamp and an Offset with the Temporal printer: https://docs.rs/jiff/latest/jiff/fmt/temporal/struct.DateTimePrinter.html#method.timestamp_with_offset_to_string

Otherwise, just serializing a Timestamp without an offset also works. It will use Zulu time. It depends on what you're trying to do.

@chmp
Copy link
Owner Author

chmp commented Oct 1, 2024

Thanks for jumping onto this issue 👍

I just read your comment on the issue re "Z" suffix and am still digesting the nuances - I have the feeling I should sit down at some point an truly understand timezones.

At this moment, I only support (naive) datetimes without timezone and datetimes with UTC timezone. The comment was about finding a common format between chrono and jiff that allows to deserialize strings encoding a timestamp in UTC to either DateTime<Utc> or Zoned.

Edit.: DateTime<UTC> does support serde see for example here

@BurntSushi
Copy link

BurntSushi commented Oct 1, 2024

At this moment, I only support (naive) datetimes without timezone and datetimes with UTC timezone.

So, there is a distinction between "datetime in UTC time zone" and "datetime in Zulu." 2024-10-01T00:00:00Z and 2024-10-01T00:00:00-00:00 are equivalent and correspond to Zulu time. The semantic meaning of these is that the offset to civil time is not known. In contrast, 2024-10-01T00:00:00+00:00 means that the offset to civil time is affirmatively 0. When you serialize a jiff::Timestamp, the default is that you get Zulu time, which means, "the offset to civil time is not known." Which makes sense. Because a Timestamp doesn't have an offset on it.

Conversely, when you serialize a Zoned, you always get an offset, because by construction, the offset to civil time is known. This means that serializing a Zoned can never result in Zulu time or -00:00.

UTC is itself not really a time zone, it's a time scale. Confusingly, it is common to use UTC as if it were a time zone (including in Jiff, which inherits this from the IANA Time Zone Database itself). But it's usually semantically wrong even though it will give the same, in practice, results. But a 0 offset can appear. For example, 2024-12-01T00:00:00+00:00[Europe/London] is a valid datetime with a 0 offset but not in UTC.

Note: "civil time" is equivalent to "local" or "naive" or "plain" time used in other datetime libraries.

Edit.: DateTime does support serde see for example here

Yes. That's not inconsistent with what I said. :-) What doesn't support Serde is chrono-tz and tzfile.

For chrono::DateTime<chrono::Utc>, the effective equivalent to that in Jiff is jiff::Timestamp, which will serialize in the same format.

Basically, you should only be supporting Zoned if you want to support the (de)serialization of datetimes with their corresponding time zones. This is a somewhat new functionality for datetime libraries. RFC 9557 was only published just this year, although java.time has supported it for some time. In many cases, systems just do not support Zoned today. For example, if you wanted a Zoned data type with Chrono, you'd have to invent your own serialization format.

@chmp
Copy link
Owner Author

chmp commented Oct 1, 2024

Thanks a lot for the detailed explanation. I will have to let it settle for a bit. Luckily, I am still fighting with the civil types ;)

@BurntSushi
Copy link

If you're interested, my handle is burntsushi5 on Discord if you want to have a more synchronous chat.

@chmp
Copy link
Owner Author

chmp commented Oct 1, 2024

Thanks for the offer. I will close the laptop for today, but will definitely ping you, once I look into this issue again.

serde_arrow/src/internal/chrono.rs Outdated Show resolved Hide resolved
serde_arrow/src/internal/chrono.rs Outdated Show resolved Hide resolved
@chmp chmp marked this pull request as ready for review October 5, 2024 07:28
@chmp chmp merged commit 73b102a into main Oct 5, 2024
3 checks passed
@chmp chmp deleted the feature/231-jiff branch October 5, 2024 07:34
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

Successfully merging this pull request may close these issues.

2 participants