jorgecarleitao / arrow2

Transmute-free Rust library to work with the Arrow format
Apache License 2.0
1.07k stars 220 forks source link

Add datetime with timezone support to Json serializer #1510

Closed noselasd closed 1 year ago

noselasd commented 1 year ago

Hello, We were trying to serialize data to Json via polars, but was hit with the missing support for DateTime types with timezone support. I'm not too well versed in Rust, but I made an attempt at adding support for it.

It's inspired by how the CSV serializer does this. The Json serializer doesn't propagate errors back in new_serializer() as the CSV serializer does, I havn't attempted rewriting the interface, so this panics if invalid timezones are encontered. It should still be an improvement over the todo!("still have to implement timezone") in src/io/json/write/serialize.rs

DateTime serializing is done differently from serializing NaiveDateTime, which uses a space instead of a T as the date and time separator. However the default chrono to_string on timezone aware DateTIme produces a space before the tz offset (e.g. 1996-12-19 16:39:57 +08:00) which Javascript engines won't parse as a Date(), so the RFC3339 format is used instead. chrono-tz timezones are also serialized with a fixed timezone offset format [+-hh:mm] so Javascrpt engines can make use of it.

I do have some future plans as well, including adding a options for custom time format string as is done in the CSV parser, some knobs to tune how NaiveDateTime is serialized, but this is perhaps a start.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 87.50% and no project coverage change.

Comparison is base (15c5ec1) 83.43% compared to head (ab2125a) 83.44%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1510 +/- ## ======================================= Coverage 83.43% 83.44% ======================================= Files 388 388 Lines 42070 42104 +34 ======================================= + Hits 35102 35133 +31 - Misses 6968 6971 +3 ``` | [Files Changed](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1510?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | | |---|---|---| | [src/io/json/write/serialize.rs](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1510?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2pzb24vd3JpdGUvc2VyaWFsaXplLnJz) | `84.65% <87.50%> (+0.90%)` | :arrow_up: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1510/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ritchie46 commented 1 year ago

@MarcoGorelli could you take a look at this one?

ritchie46 commented 1 year ago

Nevermind, looks good!