Skip to content
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

From/Into implemenation for DateTime<FixedOffset/Utc> conversion? #831

Closed
kornelski opened this issue Oct 1, 2022 · 5 comments
Closed

Comments

@kornelski
Copy link

I frequently struggle to find the right methods to convert between various types in Chrono.

Would it be possible to implement From/Into for conversion between types like DateTime<FixedOffset> and DateTime<Utc>?

@esheppa
Copy link
Collaborator

esheppa commented Oct 2, 2022

Thanks for raising this @kornelski - it is good timing as there has been some discussion and proposals surrounding this recently.

Other discussion can be found in #169 and #624. There is an ongoing PR to help this somewhat in #829. There is also some discussion around simplification/changes to the TimeZone trait may also make this simpler in #822.

All that said, with current chrono 0.4.22 your best option is:

let my_fixed_offset_datetime = my_utc_datetime.with_timezone(&Utc);

In terms of From impls - this would probably be best implemented on a subset of the TimeZone implementors, this is because while conversions between FixedOffset and Utc are trivial and cheap, once Local is in the mix, calling from() or into() could potentially hit the filesystem, allocate memory, etc which means a From impl may not be ideal (however this is also argurable, and could be just a case of documenting).

However we are keen to improve the documentation / discoverability of this (potentially also via simplification of the trait, and moving some/all of the convenience methods to TimeZone implementors - which also makes IDE discoverability better). One thing I'd like to note though is that this is documented at Date and Time in the readme, so it would be good to understand which parts of that could be made clearer or whether there are other things we could do to improve discoverability.

@kornelski
Copy link
Author

kornelski commented Oct 2, 2022

I don't think From has to be very cheap. It's only meant to be infallible and lossless (I hope there isn't some edge case that makes conversion to Utc and back impossible to do losslessly if the date was a second before midnight in eastern Kiribati in 1968 or something like that).

@kornelski
Copy link
Author

kornelski commented Oct 2, 2022

The docs at DateTime show a lot of .hms()-like methods, which don't seem like the right way to convert between types. Taking them apart into pieces like this is very boilerplatey, and I'd be worried that I'm forgetting some field or failing to properly account for timezones.

It'd be ideal to have a table with various types in Chrono, std, and other conventions like unix timestamp and string representations, and code examples for all the pairs of types.

@esheppa
Copy link
Collaborator

esheppa commented Oct 4, 2022

Thanks @kornelski - the table is an excellent idea.

I don't think From has to be very cheap.

Agreed on this, however, the problem is:

It's only meant to be infallible and lossless

Currently when doing an operation with Local time, on many platforms chrono will read a file in the filesystem, which seems out of place in a From impl

@pitdicker
Copy link
Collaborator

I don't understand this issue.
Chrono has From implementations to convert between DateTime<Utc>, DateTime<Local> and DateTime<Fixedoffset> for quite a while, since #271.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants