Open botev opened 7 years ago
+1 I could really use implementation of Serialize/Deserialize for a project I'm working on.
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 derive
s 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 derive
s would be sufficient here?
Hi @JoeyAcc, I'm learning Rust at the moment, what are the newtypes you talked about?
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?
@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.
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.
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?
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.
Chrono (at least for the 0.4.x series) will continue to support three "duration-like" types:
std::time::Duration
for the standard library compatibilitytime::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.1The 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.
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?
@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.
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.
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.)
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?
+1 I could really use implementation of Serialize/Deserialize for a project I'm working on.
Same here.
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.
I like the “total number of seconds” serialization. Nice and simple, unlike ISO 8601 periods.
I would stay on the standard as a default, and use the with
attribute for different kinds of de/serializations
Support for a human readable format would be helpfull for config files. Something akin to humantime.
the time crates has now a serde features too, chrono could just update time dependencies.
+1
@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?
+1
+1
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.
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:
time
to 0.2
(https://github.com/chronotope/chrono/issues/553)namespaced-features
(https://github.com/rust-lang/cargo/issues/5565)serde = ["dep:serde", "time/serde"]
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.
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
derive
s 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 ofderive
s would be sufficient here?
Which types do even support Serialization atm?
Any update on this?
I think we're ready to do this -- anyone want to send a PR?
I also would love if (DateTime<T>, TimeDelta)
would implement Serialize and Deserialize such that it is compatible with iso8601 (as intervals, meaning either startdate+duration, startdate+enddate or duration+enddate, and default to startdate+duration for the Serialize impl or something)
Maybe something like this:
I'm not sure where to copy and paste this ...
pub mod timedelta_milliseconds {
use chrono::TimeDelta;
use serde::{Deserialize, Deserializer, Serializer};
pub fn serialize<S>(duration: &TimeDelta, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_i64(duration.num_milliseconds())
}
pub fn deserialize<'de, D>(deserializer: D) -> Result<TimeDelta, D::Error>
where
D: Deserializer<'de>,
{
Ok(TimeDelta::milliseconds(i64::deserialize(deserializer)?))
}
}
pub mod timedelta_milliseconds_opt {
use chrono::TimeDelta;
use serde::{de, Deserialize, Deserializer, Serializer};
pub fn serialize<S>(opt: &Option<TimeDelta>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
match *opt {
None => serializer.serialize_none(),
Some(dur) => serializer.serialize_some(&dur.num_milliseconds()),
}
}
pub fn deserialize<'de, D>(deserializer: D) -> Result<Option<TimeDelta>, D::Error>
where
D: Deserializer<'de>,
{
deserializer.deserialize_option(OptionMillisecondTimeDeltaVisitor)
}
struct OptionMillisecondTimeDeltaVisitor;
impl<'de> de::Visitor<'de> for OptionMillisecondTimeDeltaVisitor {
type Value = Option<TimeDelta>;
fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result {
formatter.write_str("Some(milliseconds) or None")
}
fn visit_none<E>(self) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(None)
}
fn visit_some<D>(self, d: D) -> Result<Self::Value, D::Error>
where
D: de::Deserializer<'de>,
{
let millis = i64::deserialize(d)?;
Ok(Some(TimeDelta::milliseconds(millis)))
}
fn visit_unit<E>(self) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(None)
}
}
}
maybe somehere in https://docs.rs/chrono/latest/src/chrono/lib.rs.html#612
For features
serde
andrustc-serialize