Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fix Timestamp.round errors #22802
Fix Timestamp.round errors #22802
Changes from all commits
fb25c21
a8ded73
eee47a6
f6f131b
53c8c9b
988e5e3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there round errors in timedelta ops as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, also
timedeltas
are affected by rounding errors, but it is very unlikely that these will be found in real world applications.BTW I found #22591 processing real scientific data, in which I had timestamps for 2018 measurements with about 1 µs resolution. On the contrary I cannot imagine of an application in which you have 50 years time deltas with such a small resolution.
Of course you could create a round-trip example:
which gives different results with respect to
Nevertheless I would suggest to postpone the resolution of the timedelta issue after this PR is merged.
(Please note also that the current complex machinery, which tries to improve rounding errors, is implemented only for Timedate and not for Timedelta.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no i get it, that's why I asked. Ok to fix later, esp if its not very common.