-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
serde support: TimeZone
or Offset
#89
Comments
Can you say more about why you're serializing these types separate from the datetime types? |
I'm building a Discord bot for friends and want to keep things simple by tying them with their time zones. Since I'm already using Please let me know if that helps enough. I'm happy to give code examples! :) |
Right, so you should be able to use But reading closer, it looks like you want to associate the time zone itself with the user outside the context of a datetime. Which I think makes sense. Can you say though why you want to serialize an offset instead of just a time zone? In terms of technical implementation, there isn't any problem with adding a serde impl for But for Instead, I would ask you to consider storing a IANA time zone identifier for each of your friends. It might be missing if you don't know it. Then you would just call Does that help? |
Great, thank you! My idea for using an Currently, I do this by storing a /// A representation of someone, with some of their details important to
/// knowing when to ping.
#[non_exhaustive]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct Person {
user_id: UserId,
timezone: Offset,
/// A list of timespans where they're unavailable (gone).
gone: Vec<Unavailability>,
}
impl Person {
/// Given a `UserId`, creates a new person with "default" availability
/// and timezone (i.e. both blank).
pub fn new(user_id: UserId) -> Self {
Self {
user_id,
timezone: Offset::UTC,
gone: Vec::new(),
}
}
}
/// A span of time where someone isn't able to play.
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct Unavailability {
span: Span,
} For this code to work, I need the user's timezone to be stored independently of their availability. Otherwise, I can't convert their spans if they change their timezone. Using It's also less fallible, as I don't need to 'convert' the list of Spans to the timezone each time I try to use them. (If I didn't do that with a Timezone IdentifiersI wouldn't mind storing a timezone identifier, so long as I can easily find them. Can I get it from a given Would it make sense to store a string? Would it be safe? Thank you! |
My take is that you should 100% be storing an IANA time zone identifier. And yes, it should be stored as a string. And no, you cannot get an IANA time zone identifier from an offset because there may be many IANA time zone identifiers for the same offset. Instead, it's the reverse: you get an offset from a time zone and either a Using an IANA time zone identifier actually seems critical to your use case here, to the point that storing an offset would be completely wrong. For example, for where I live, my IANA time zone identifier is always |
Storing an offset is definitely wrong to some degree! To address the issues you mentioned, I'll probably make a serde-impl'd type that holds a String and interfaces with Nonetheless, I'd love to see some form of serialization. So, assuming you'd want to serialize in IANA form, that leaves just two questions:
Answering these questions would solve this issue. However, for my simple project, using String timezone identifiers and |
No, it can't. That's why As for exposing the inner TZif, it is in theory possible, but it's probably a bad idea. I'm more likely to go the route outlined in #30. But making that "easy" via a serde impl is almost certainly the wrong thing to do. |
I tend to agree on the The plan in #30 sounds pretty good! Please let me know if you see any movement there. I'll watch the issue in the meantime. |
Going to close this out in favor of #30. (Although, from my perspective, it does seem like just an IANA tz identifier is sufficient here for the OP's use case.) |
@BurntSushi I'd like to be able to store That said, from an application development perspective, there is no public type that says "this can only be IANA". So when going to implement a library that uses If there's a better way of doing this already built into |
I think you probably want to accept a Also, if you're storing zone aware datetimes in a DB, you can use the Display and FromStr impls on Zoned. They will do the right thing, including attaching the IANA zone name if it exists. If this doesn't help, can you suggest an API change in Jiff that does what you want? If only as a way to crystalize what it is you're looking for and prevent misunderstanding. |
I still wouldn't have a way to store the I should also point out, if a user wants to use a non-IANA Because users of this library will already be using
The elephant in the room is |
Can you say more about your higher level use case? i.e., more detail than just that you want to store it in a database. |
I'm not sure what additional detail you're looking for. Can you be more specific? |
"storing in a database" is a means to solve a problem and not an end itself. So, what problem are trying to solve here? When dealing with datetimes, and especially in contexts that motivate the API design of a datetime library, it is really important to understand the higher level use case. |
The design of a library that uses Is it possible to convert from a string to a Taking a step back, while I agree that often IANA-names are "what you really want" I don't agree that it's correct to force that decision and the argument doesn't land as well when there's really no type for an IANA-only |
I appreciate the response. I do think it's going to be difficult to make progress here without concrete use cases. I recognize you probably feel like you've given enough detail, but it is very difficult for me to think about this in the context of the details you've given so far. I'll re-open this issue for now. Let me try this again: can you suggest an API addition or change to Jiff that would accommodate your use case? I asked this before but I don't think I really got a precise answer to this. |
I can think of a few possibilities:
I'm happy to talk about more concrete use cases, but I do want to avoid reducing the conversation to "just use strings": that isn't the design path I'm hoping to pursue. It's also not technically correct for me to impose IANA names, for e.g. running schedules that don't care about DST awareness and instead are just as validly defined with an offset. So the most correct answer is the user should decide by providing a That said, I would accept |
Wait what? The most important types in this crate are the datetime and duration types. All of them implement both Serialize and Deserialize when the serde feature is enabled.
But how do they work? This is the key thing missing. A This is exactly why I asked this question because I suspected there might be some confusion in the specific concrete details.
How would this type be used in Jiff? The problem is that a Alternatively,
That's what #30 is intended to be. Or am I misunderstanding? Note that |
The overall idea here is that a This is why the use case is important. If you want to serialize the time zone in a compact offset-or-iana format, then it probably makes sense for your library to define a more limited type representing that choice and then maybe convert to Because we haven't been able to talk about use cases, the thing I still don't understand is why a |
Then we badly need I think it's also worth noting that
If you thought that the only useful representation was a timestamp, you probably wouldn't have made
Then I think it pretty clearly follows that the ability to store this and only this is useful. There's a fundamental contradiction between this and the previous sentence: a configured time zone has nothing to do with a timestamp per se and I certainly don't want to store some arbitrary timestamp just so I can capture the time zone.
I would accept a type that's either IANA names or offsets; a subset of the internal kind enum for example. Either are valid for many of the use cases where you might need to store time zone data in my view. |
Not really haha. I spilled a fair bit of words on this topic in the Jiff design and comparison docs. Chrono's implementations of its TimeZone trait in chrono proper all support serde, but none of those correspond to IANA time zones. There is the chrono-tz and tzfile crates which provide their own TimeZone trait implementations, but neither of them support Serde. Basically, you're running into a shortcoming of Chrono that let you easily do what is arguably the wrong thing in most cases (since you lose DST info). That gave the appearance of an additional capability, but in reality it's more complicated than that.
It's a lot more than two... It's not like it's an arbitrary choice or something. I think the only type that doesn't support serde at present but easily could is
I find this to be a somewhat frustrating response. I'm trying to tell you something I don't understand. I'll add my intent more explicitly: can you help me understand it better?
That's not what I said though. I tried to be very careful with my wording, but perhaps I failed. Let me be more precise: given any arbitrary
Okay, I didn't say that... What I mean is that you might find it easier to define a more limited time zone type that is specific to user configuration. A Jiff
I understand this, but we're going in circles. My previous comments asked for more details about this option. My sense is that a type like this would be better defined closer to the user configuration than in Jiff proper. |
My previous comments also provided some very specific concrete technical problems with your suggestions. But the conversation is veeeing back towards abstractions. Can we try to keep it concrete so that we can drive toward a specific solution to your problem? |
Something about this is not right: I have a bunch of IANA names serialized to Postgres using My suspicion is that the "TimeZone" type is overloaded and that With that, I unfortunately don't have more to add here that I haven't already and I'd like to leave space for other folks to chime in. |
Ah okay, the Tz type itself supports Serde. But
There are trade offs involved here. A Jiff time zone is in fact a single conceptual thing: it is a transformation mapping between civil times and instants via offset. Chrono exposes bits and bobs of this concept but provides no cohesive type. As a result, there is more flexibility in Chrono's design and it is easier to do incorrect things, or at least, harder to do the right thing. But not all abstractions are created equal. Jiff for example provides an abstraction over "time zone data embedded in binary" and "tome zone data read from zoneinfo on disk." Chrono doesn't give you that. You, the library user, have to assemble that logic yourself. It is true that a Jiff time zone is one of many possible things, but each of those things corresponds to a time zone that is used in the same way. And its API is closed, meaning you can't extend the concept in your own code (unless you provide TZif data). Conversely, in Chrono, it is an open concept at the level of the type system. So each different kind of time zone is exposed and users can extend it with their own implementations of that trait. I think it is a good idea to have a layer of abstraction between "end user configurable time zone" and "jiff time zone that is used to map between between instants and civil times via offsets." They just aren't the same thing. But I'm not quite seeing why the former should be in Jiff. It seems like something that should be defined specific to the application or use case. For example, you might not even ask an end user for a TZ id. Instead, you ask for a location and derive the IANA TZ id from it. |
I'm re-reading the discussion now, especially the beginning. I think there are more questions to ask here:
Can you say why, more specifically, accepting a
I should have caught this earlier, but I actually think this might be the crux of the issue. Because "I want to accept any kind of I would really like to help you drive toward a solution here. I am very interested in trying to understand use cases better so that Jiff becomes an obvious choice for you. And especially now before Jiff 1.0 where I have a bit more freedom to do breaking changes. If you'd like to work with me on this to help me better understand what you're trying to do, that would be great. |
Hey there! Thanks for this incredible new crate!
I'm using it in a new project, but I'm having difficulties due to the
TimeZone
andOffset
types lackingserde
implementations. I can understand why aTimeZone
might not have it (the design document was very convincing).However, is it intentional that
Offset
doesn't have one? This seems like it'd just serialize that internal integer. (Thedefine_ranged!
machine does look mildly complicated here, of course).Please let me know if I'm missing anything, or there's something else that might be better for serializing a user's timezone.
Once again, thanks for the new contribution to the ecosystem! :)
The text was updated successfully, but these errors were encountered: