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

Truncating a timestamp close to EPOCH fails #1375

Closed
joroKr21 opened this issue Jan 18, 2024 · 6 comments · Fixed by #1403
Closed

Truncating a timestamp close to EPOCH fails #1375

joroKr21 opened this issue Jan 18, 2024 · 6 comments · Fixed by #1403

Comments

@joroKr21
Copy link
Contributor

joroKr21 commented Jan 18, 2024

Due to this check in duration_truc:
https://github.com/chronotope/chrono/blob/main/src/round.rs#L221-L223

Expected behaviour: I should be able to truncate the timestamp to 0

@djc
Copy link
Member

djc commented Jan 18, 2024

I'd be happy to review a PR -- I probably won't have time to work on this myself.

@joroKr21
Copy link
Contributor Author

It's not urgent for me but I think I will find time for it

@pitdicker
Copy link
Collaborator

To be honest in my opinion the API and approach in the round module is fundamentally flawed. A lot of the behavior doesn't make sense, especially for a library that tries to do handle dates and timezones correctly like chrono. And I don't think it is even possible to fix all logic bugs.

But we would need to have an alternative before it can be deprecated.

@djc
Copy link
Member

djc commented Jan 26, 2024

But we would need to have an alternative before it can be deprecated.

Why? If we think this API is silly or wrong or dangerous, we could deprecate it without providing an alternative.

Does the time crate provide this functionality?

@pitdicker
Copy link
Collaborator

There are some cases where it works and gives correct results. For example rounding or truncating a NaiveTime to a reasonable Duration such as 10 seconds. But rounding to for example 7 seconds, and especially when involving dates becomes a mess. And that is ignoring DST 😄.

Why? If we think this API is silly or wrong or dangerous, we could deprecate it without providing an alternative.

Maybe you are right. It does not really feel like an inconvenience yet, but we can expect more issues like this.

Does the time crate provide this functionality?

It doesn't seem to.

@pitdicker
Copy link
Collaborator

Another issue with duration_trunc: #584.

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

Successfully merging a pull request may close this issue.

3 participants