Add Serialize/Deserialize for Duration

[botev]

For features serde and rustc-serialize

[stusmall]

+1 I could really use implementation of Serialize/Deserialize for a project I'm working on.

[JoeyAcc]

I wholeheartedly agree. In fact, i'd like to see serialization support for all the core types: Date, Time, DateTime, UTC, Local etc.

At the moment I need to define newtypes in my own code that basically do nothing except handle Serde support. It would be absolutely wonderful to just scrap all that serialization code, as it's rather boilerplate-heavy, and thus helps turn otherwise-nice code into a "Can't see the forest for the trees" kind of deal.

If this issue can be taken care of by adding derives to the relevant structs, I'd be willing to create a pull request with the changes. If not, there will be too many (potentially gnarly) time-related details to quickly get the job done, and thus I'd rather leave it to someone who already has the necessary knowledge about de/serializing times and dates. Does anyone know whether a couple of derives would be sufficient here?

[Geobert]

Hi @JoeyAcc, I'm learning Rust at the moment, what are the newtypes you talked about?

[dylanede]

I would also really appreciate support for (de)serialising Duration. But it seems that that type comes from the time crate, which is deprecated. What's the situation regarding the dependency on time, @lifthrasiir?

[JoeyAcc]

@Geobert I only now see this response, sorry about that.

A newtype is a wrapper type around one other type, for details have a look at this.

How that plays out in this context is that you write such a newtype N for e.g. Duration, and then implement Serialize and Deserialize for N rather than Duration (which is not possible as both it and Serde's traits are defined outside your own crate).

By manipulating exactly how the internal Duration is de/serialized (which is done by manually implementing Serialize and Deserialize) it's possible to get around the whole "I can't serialize this!" issue, at the cost of a fair amount of boilerplate.

That is also where my request to implement serialization directly for all chrono types comes from.

[quodlibetor]

This would need to be implemented in rust-lang-deprecated/time to be done correctly. I believe that @lifthrasiir is working on writing a new Duration for chrono itself, which will make it possible for us to do this trivially.

For now you could write your own deserializer, see my comment here for an example of how.

If you feel like doing a particularly good job I would take a PR that implements that in chrono, keeping in mind that we're getting rid of the Duration before 1.0.

[JoeyAcc]

If I understand you correctly, by "getting rid of the Duration" you mean the impl from rust-lang-deprecated/time? In other words, from an API perspective it wouldn't be removed so much as completely replaced by another type that has the same Duration name?

[quodlibetor]

Yes, mostly. A new Duration will be created entirely within Chrono. We will probably continue to export the OldDuration struct for one or two releases before it is deleted and only the chrono-internal Duration exists, but I'm not sure what the plan actually is.

[lifthrasiir]

Chrono (at least for the 0.4.x series) will continue to support three "duration-like" types:

• std::time::Duration for the standard library compatibility
• time::Duration for the backward compatibility (this is still mandatory, but the plan is to make the time dependency optional with the introduction of TimeSpan below)
• chrono::TimeSpan which will be the new signed duration-like type, planned for 0.4.1

The new TimeSpan type will be used for the "natural" date and time computation then. I've almost finished the design of TimeSpan and am filling the gaps in the internals.

[JoeyAcc]

Thank you both for the explanations. I have one more question: Will the new TimeSpan type be backed by a similar second + nanosecond field construction as Duration or will it use the new u218 or i128 types that are in the process of being stabilized?

[lifthrasiir]

@JoeyAcc It will be essentially the same format as std::time::duration except for being signed. There is no reason to use 128-bit integers for TimeSpan because it is simply too large to be useful---even 64-bit integers in the nanosecond precision will suffice for more than 500 years. The main reason to split the seconds (64 bits) and nanoseconds (32 bits) is that it is more efficient for most use cases, as many APIs and data structures do not directly count the number of nanoseconds.

[JoeyAcc]

I see, that makes sense. I was not aware of those restrictions on the design space, so I kept wondering why have the sec/nanosec split at all. At first I thought it was due to the value range of u64/i64. But as you already remarked, The range is sufficient for a couple of hundred more years*. Thank you for explaining this to me.

*This is assuming the hardware stays at nanosecond precision, which may not hold. I myself wouldn't mind moving as far as we can in the direction of the Planck time unit, for example.

[lifthrasiir]

This is assuming the hardware stays at nanosecond precision, which may not hold. I myself wouldn't mind moving as far as we can in the direction of the Planck time unit, for example.

In that case we can always make TimeSpan::with_attos(secs, nanos, attos) and extend the TimeSpan to have another u32 field :-) (This method of extension can be seen in, for example, TAI64.)

[Procrat]

I'm not entirely following the history of this issue, but I noticed that the dependency on the time crate is optional now and that the implementation of Duration is ported to this crate. Does that mean it is now actually possible to solve this issue?

[matthiasbeyer]

+1 I could really use implementation of Serialize/Deserialize for a project I'm working on.

Same here.

[quodlibetor]

Yeah now that we are working on completely owning time I agree with getting an implementation for this in.

My inclination is to emit something like SECS[.FRAC] as the default, but maybe something like ISO8601 periods would make it more obvious that this is intended to be a duration? We can always add serde helpers to make the non-default case reasonably pleasant. Another option would be something like python's HH:MM:SS, which is a bit more pleasant to read at the expense of being kinda misleading.

[fenhl]

I like the “total number of seconds” serialization. Nice and simple, unlike ISO 8601 periods.

[garro95]

I would stay on the standard as a default, and use the with attribute for different kinds of de/serializations

[jean-airoldie]

Support for a human readable format would be helpfull for config files. Something akin to humantime.

[Stargateur]

the time crates has now a serde features too, chrono could just update time dependencies.

[giacomocariello]

+1

[Exr0n]

@quodlibetor I'm a bit of a rust newbie, but I'd like to give a shot at implementing the traits. I saw your linked comment, but I'm not sure where to start. Can you give me some quick pointers?

[0x8f701]

+1

some infos that I found:

• https://github.com/jean-airoldie/humantime-serde

[mosesonline]

+1

[leontoeides]

+1

[timvisee]

The current Duration type seems to be: https://docs.rs/chrono/0.4.19/chrono/struct.Duration.html

Is there any reason for it not supporting Serialize/Deserialize yet other than development time? Are there implementation details I should know about? Because I'd be happy to take this.

Edit: looking into this now.

[timvisee]

I did some investigation, here are my findings:

• chrono still uses the Duration type provided by the time crate.
• chrono currently uses time 0.1, which doesn't provide serde support for Duration.
• time 0.2 does support serde for Duration, behind the serde feature.

So to fix this, time must be updated to 0.2 first.

Then, the serde feature in time must be enabled. This is tricky, because chrono provides the serde feature and crate. You can't specify a feature with the same name as one of your crates in the current Rust stable version due to name collisions. This can be fixed with feature namespaces.

In other words, we'd like to specify the following feature in Cargo.toml but feature namespaces (notice dep:) are yet to be stabilized:

[features]
# --- snip ---
serde = ["dep:serde", "time/serde"]

There are two alternatives for this to choose before stabilization. The first is to enable serde by default for time. The second alternative is to provide a feature such as time-serde = ["time/serde"]. We probably don't want either of these alternatives for various reasons.

---

So here are the steps I suggest:

TODO:

• Update time to 0.2 (https://github.com/chronotope/chrono/issues/553)
• Stabilize namespaced-features (https://github.com/rust-lang/cargo/issues/5565)
• Add serde = ["dep:serde", "time/serde"]

[TimYorke]

Unfortunately this needs revisiting - #553 has been closed

[djc]

Similarly to the additional serde formats we support for things like DateTime, I'd be happy to review a PR that implements custom serializer/deserializer modules for the Duration type. Given how they're implement (using serde's with annotations), I don't think these would run into coherence problems.

[nyovaya]

I wholeheartedly agree. In fact, i'd like to see serialization support for all the core types: Date, Time, DateTime, UTC, Local etc.

At the moment I need to define newtypes in my own code that basically do nothing except handle Serde support. It would be absolutely wonderful to just scrap all that serialization code, as it's rather boilerplate-heavy, and thus helps turn otherwise-nice code into a "Can't see the forest for the trees" kind of deal.

If this issue can be taken care of by adding derives to the relevant structs, I'd be willing to create a pull request with the changes. If not, there will be too many (potentially gnarly) time-related details to quickly get the job done, and thus I'd rather leave it to someone who already has the necessary knowledge about de/serializing times and dates. Does anyone know whether a couple of derives would be sufficient here?

Which types do even support Serialization atm?