-
Notifications
You must be signed in to change notification settings - Fork 103
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 an OffsetDateTime type #90
Comments
It would probably be useful to use an |
conversion from and to import kotlinx.datetime.toJavaLocalDateTime
import kotlinx.datetime.toJavaZoneOffset
import kotlinx.datetime.toKotlinLocalDateTime
import kotlinx.datetime.toKotlinZoneOffset
public fun java.time.OffsetDateTime.toKotlinOffsetDateTime(): OffsetDateTime {
return OffsetDateTime.of(
toLocalDateTime().toKotlinLocalDateTime(),
toOffsetTime().offset.toKotlinZoneOffset()
)
}
public fun OffsetDateTime.toJavaOffsetDateTime(): java.time.OffsetDateTime {
return java.time.OffsetDateTime.of(dateTime.toJavaLocalDateTime(), offset.toJavaZoneOffset())
} |
So did I get it right that you need such a type to represent deserialized values of the built-in date-time OpenAPI format? |
So not only specifically for OpenAPI, but in general to represent RFC3339 Datetimes (which is meant to be the Internet standard). |
One specific use-case - our API deals with public transport departure times (bus, metro, etc) in various cities. Public transport departure times (like flights) are always displayed in the local city time. The API returns times conforming to RFC3339 such as This does not imply that it is necessary to have a |
FWIW: Crystal's equivalent of |
Came here searching for a multiplatform replacement for java.time.ZonedDateTime. It's really surprising that kotlinx-datetime isn't equally able to parse an ISO-8601 and/or a RFC-3339 date string that includes a non-zulu time-zone. Or am I getting it wrong? |
|
But parsing it to an instant the time zone is getting lost isn't it? |
The parsed time zone is not returned to the user, yes. |
Which does result in the loss of information |
Then unfortunately this is not a solution / fix for us, as we have to keep the time zone. |
We do want to support returning the parsed offsets, but the exact implementation is unclear: maybe it's sufficient to just return a To decide this, we need to consider the specific use cases. @harry248, could you please share yours? @justasm already mentioned that an API they're dealing with returns the instants with a time zone that also indicates how the instant should be displayed to the end-user—this is a good point, thanks! |
@dkhalanskyjb It's exactly the same scenario in our case. |
My use-case for |
I've had one (rare & weak) use case for That use case is more or less #90 (comment) where also the API isn't providing an ideal data format. It should probably provide a local datetime + time zone instead of datetime merged with an offset. In general over many different projects there hasn't been any instance where I've needed An offset I've never needed except for the very rare use case mentioned initially. |
Our use-case is to be able to losslessly serialize/deserialize and work with arbitrary future datetimes with zone and DST information like
Only (3) can correctly represent a future event/datetime if the zone's offset and DST handling gets changed unexpectedly. Using separate Finally, with such a flexible |
Hi @wkornewald!
Could you elaborate on this? I don't see why this is true. Consider two cases here:
Am I missing some case here where In general, IIRC, this is the first time someone asks that we handle the DST in a specific manner. |
Note that point (3) is just an edge case to be able to represent a precise umanbiguous clock readings time. My main suggestion is to have a ZonedDateTime that at least fulfills (1) and (2), but additionally being able to losslessly represent serialized string-based dates produced by java.time would be nice. Regarding (3): If you want to trigger an event at an exact point in clock readings time then you have two different interpretations. How do you distinguish the point in time before the DST change vs. the time after the change? 02:30 could mean two points in time. |
Tricky.
|
Imagine a „simple“ calendar app that wants to allow users to specify the DST flag when an event occurs at some ambiguous time. The exact day or time when the switch to DST occurs might change in the future and an Instant can’t deal with that, so the only option would be ZonedDateTime with a nullable DST Boolean (part of TimeZone?). |
But you're right, this is pretty obscure and indeed maybe not worth it - though then we can't be fully compatible with everything representable by java.time then. Anyway, my main request was having a ZonedDateTime where the zone can be either an id or an offset and I just continued to think of obscure edge cases and then the DST question came up, so I compared how java.time handles that. We're discussing the DST point too much and let's better focus on the core ZonedDateTime aspect. 😄 |
Can't really imagine that. I'd expect the calendar app to provide a time picker, which, if the author of the calendar so chose, would have some times repeated.
Oh, I think it's not a viable goal at all, given how vast the Java Time API is. We want this library to be simple and discoverable, not all-encompassing.
Ok, sure. In your case, since, it looks like, you can choose the format in which to send data, right? Then, I think a good choice depending on the circumstance is either an |
I think aligning with java's behavior makes a lot of sense instead of rolling your own and dealing with a lot of inconsistent behavior between different services.
This seems to basically be how Java implements it and is also how I work around it right now. |
I can't fully choose the format because we have an existing system and already pre-existing official specifications that we have to support. Actively juggling with LocalDateTime and TimeZone as separate values is definitely not enough.
We had to build our own ZonedDateTime to deal with this and I'm pretty sure everyone else is doing the same thing because this is a very common use-case. Other libraries already provide it out of the box and I think this library should do it, too. |
If you want to keep the API as small as possible then here’s some provocative food for thought: If you had ZonedDateTime instead of Instant maybe people wouldn’t be missing Instant because ZonedDateTime in UTC handles that use-case already. The question is where you want to make distinctions explicit via types and maybe where you want to provide common base types to allow abstracting the distinction away. |
The use-case for instant is different: It should be use to record past events that may not change (e.g. the Unix timestamp cannot change) while ZonedDateTimes can (e.g. if laws change), which is why they should be used to record things like future appointments, so maybe not impossible to use a common type, you have to be extra careful to not break those functionalities. |
Here are the two use-cases, I also mixed those, so we should probably split it to clear it up:
|
@ilya-g I always tried to explicitly say that the ZonedDateTime would handle offset and ID dynamically based on the amount of information available. And depending on how precise we want to represent time we could also still consider even a combination of both offset and ID at the same time. Basically, what I’d like to see is an object that can precisely identify the local and global time at the same time and adapt to the precision that is available. |
@cromefire To add to above,
|
I think
I think comparative operations ( |
@cromefire Consider a range The natural order of OffsetDateTime breaks an intuitive expectation that the result of comparison of two OffsetDateTimes should be the same as the result of comparison of Instants they correspond to. That's why in Java, OffsetDateTime additionally has functions |
I would say no for the first. And yes for the second (assuming inclusive). (all assuming I calculated correctly in my head and basically just comparing the absolute points in time (e.g. UTC / Instant))
Well I could certainly agree with having having explicit methods instead of operators, might make more sense. So I think they are good for representing a series of events in a timeline, but yes I think we could put in explicit methods instead of doing something implicit. |
So do we have a preference already? Maybe I should summarize the points again because after @cromefire's summary you both made some additional important points. Support in practiceDepending on the database, API and official standards you might have to work with either.
Of course at the database level, depending on your DB choice, you could use two separate fields if the level of precision is important. With third-party APIs and official standards you have no choice, so this means offsets most of the time. Precision and correctness / lack of ambiguity:This is mostly about mapping 1:1 to Instant.
Neither is 100% precise and unambiguous all of the time. Only the combination of offset + zone id ( Comparisons and arithmeticHere it's not always enough to just map 1:1 to Instant. You need to know the time zone rules.
What's the solution?This is my personal wish list:
Overall, this would still make ZonedDateTime just a relatively simple wrapper around LocalDateTime + mandatory TimeZone (= RegionTimeZone or FixedOffsetTimeZone) + optional DST offset. |
This makes basically nothing better, because now you have to choose which one to use if they conflict, which isn't really that much better... The only thing it would help with is having just a normal offset and using the timezones just to display the timezones to the user, but I don't think one would really need a type for that. There's also no standard I know of that uses this.
Well because an offset isn't a time zone and isn't trying to be one it doesn't know DST by design. There might even be multiple time zones per offset, it's an irreversible conversion. It a pretty severe limitation of the format. To proceed in general I'd recommend fast-tracking |
A DST offset instead of UTC offset should be able to solve this, though. In case of an invalid DST offset you just fall back to the only possible time at that point.
Then I'd rather suggest a ZonedDateTime that takes a TimeZone (i.e. either RegionTimeZone or FixedOffsetTimeZone). This way you're at least flexible and you can represent the future correctly. And this also allows to still add an optional DST or UTC offset at a later point without breaking anything or introducing yet another DateTime class. |
Yes, that's how most implementations of something like that seem to work, I man supporting both types isn't much more complicated than But let's pause this here and implement |
Mostly the parse() and toString() methods and arithmetic with durations need special handling based on RegionTimeZone vs. FixedOffsetTimeZone. I think the whole rest of the class could be identical for both cases. |
But why do you need OffsetDateTime if you can represent them with an almost equally simple ZonedDateTime? I mean, why should we have multiple classes which only confuse other developers? |
Because it's a much more (on purpose) constrained and a more simple class. I mean why do we want UInt when we have Int? It's artificially constrained and simpler, and that makes life easier. Also see my list above. It can provide certain guarantees that |
Hmm maybe we could later introduce a sealed interface/class for the TimeZone base type and have the two specific types derive from that, so if you parse a random string you work with the base type but if you want to enforce an offset or zone id you can work with the subtypes. |
So basically you'd want the java |
No I want a ZonedDateTime (with subtypes OffsetDateTime and RegionDateTime - sharing like 90% of the API) that can contain either an offset or a zone id so I can process an arbitrary input (imagine the JSON data can have either an offset or zone id) which gets converted to the more specific subtype (which I could match with a when expression or just keep using the base type), then work with that and format it back to the original input format without any loss. And if I explicitly want to only allow an offset I instead use the subtype‘s parser which only allows offset. |
Well that's what I meant, I don't think they will share much API if you don't build something super generic. You should use a IMO it's like having |
Temporal is much more arbitrary. No matter if you use an OffsetDateTime or RegionDateTime, the base type ZonedDateTime is a LocalDateTime with offset information (even if it's a zone id based RegionDateTime you can get the offset). This is a lot of information which means you can safely
There are also minimum guarantees:
In practice you could say that it's very useful to humans and it's also the way humans would think about these date strings if they just read them. Temporal is much worse because you don't even know if it's a date or time or instant. That's too arbitrary. And I can tell from experience that sometimes date fields can have even more precision differences and in one object contain just the year while another has a zoned date time for the same field (or year-month or date etc.) and we have to deal with that somehow and do comparisons and ordering and human-friendly formatting. Still the result is pretty useful to humans as long as you don't mix in a time-only value (which is why Temporal is useless). As long as you build an information hierarchy which strictly adds extra information at each step you can do useful stuff at each information level. |
This argument can be used to show that java.time.ZonedDateTime.parse("2021-12-15T10:38:25.793402647+10:00[Europe/Moscow]").toString() ==
"2021-12-15T03:38:25.793402647+03:00[Europe/Moscow]"
These can be parsed already with
When is I carefully read all of the above and noticed some things that (I think) are technically incorrect. Let me try to also summarize the state of things.
|
We deserialize the JSON from the backend and serialize it to the DB and I'd personally prefer to always store 100% exactly the original information instead of Instant which means loss of information. In a PR review at least I'd mark the use of Instant as future bug potential - even if right now it might not cause any problems. The future can bring crazy new requirements. I'd like to emphasize that unintended loss of information is a VERY BAD thing and causes very difficult bugs or impossible problems later. No matter in which context, you need to be careful about keeping the original information content.
Just a nitpick: if the time zone DB can get refreshed while the process is running you need LocalDateTime + TimeZone because otherwise you lose stability / reproducibility under serialization.
Do you have an example? My impression is that it adapts the Instant to ensure that the LocalDateTime+TimeZone is stable. Even in the ZonedDateTime source code it's """ To be even more precise, it's actually LocalDateTime+Offset or LocalDateTime+ZoneId depending on what input you provide. So Java's ZonedDateTime is a supertype of, let's call them, OffsetDateTime and RegionDateTime (zone id).
I actually dislike that behavior. From an end-user point of view the Instant is what matters for comparisons and ordering. For special sorting use-cases one can still construct an artificial ordering, but I don't remember ever needing this.
The same argument can be made for date + time. Still we do it because otherwise things become messy (how do you even serialize into a single field if they're separate?).
That can be exactly what you want because people in the affected country keep thinking in LocalDateTime, not in Instant. If a country shifts its base zone offset (which does happen) or moves the DST switch around then your calendar events (meetings, concerts, etc.) should stay unmodified in LocalDateTime and auto-correct the underlying Instant because people in that country keep going to work at the same LocalDateTime and even airports might adapt their active hours (some disallow flights after midnight for example). That's why Java's ZonedDateTime is not Instant+TimeZone, but LocalDateTime+TimeZone. For the past a ZonedDateTime can be mapped 1:1 to Instant+TimeZone because zone information should never change for the past. ZonedDateTime additionally handles the future more correctly. That's the difference.
That's not correct. It's pretty reliable at representing LocalDateTime and that's the intended use-case. This is also why it's not duplicating the functionality of Instant and like I said above it's very important to prevent loss of information under serialization. The only valid comparison that is left is
Oh the irony.😄 Sounds like we agree. |
Yep, sure. ZonedDateTime.of(LocalDateTime.parse("2021-03-27T02:16:20"), ZoneId.of("Europe/Berlin")).plusDays(1).plusDays(1).toString() ==
"2021-03-28T03:16:20+02:00[Europe/Berlin]" Because an intermediate computation (
Not the case. I've shown before that, on deserialization, the
It can't because 1) waiting for an hour at
Your argument: "A is good, and ZonedDateTime is good, therefore ZonedDateTime is A", whereas my argument is "ZonedDateTime is not A, A is good, therefore ZonedDateTime is not good". It's easy to observe that
Not true. |
Use #163 for |
FWIW, in the design of the new Temporal date/time API in ECMAScript (aka JavaScript), we opted not to offer an The reason we didn't offer There are valid use cases for In Temporal, we allow developers to create Note that I'm not suggesting that Kotlin needs a |
it would be quite good if there was a type (maybe let's call it
OffsetDateTime
to distinguish it from a possibleZonedDateTime
) that can represent aLocalDateTime
andTimeZone
ZoneOffset
together. This is especially useful for parsing RFC3339 compatible strings in common code (e.g. OpenAPITools/openapi-generator#7353 (comment)).It can easily be accomplished without any platform-specific code, I've already drafted a small example with should be enough for most use-cases (but uses some custom parsing logic, which will only work for RFC3339 compatible strings)
Example
The text was updated successfully, but these errors were encountered: