-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Timestamp.round precision error for ns (#15578) #15588
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15588 +/- ##
==========================================
- Coverage 91.06% 91.04% -0.02%
==========================================
Files 137 137
Lines 49307 49315 +8
==========================================
- Hits 44899 44897 -2
- Misses 4408 4418 +10
Continue to review full report at Codecov.
|
pandas/tslib.pyx
Outdated
if self.tz is not None: | ||
value = self.tz_localize(None).value | ||
else: | ||
value = self.value | ||
result = (unit * rounder(value / float(unit)).astype('i8')) | ||
if time_unit == 'N': |
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.
instead of this way I think you want to deal with offset.nanos < 1000 and offset.nanos % 1000 != 0
.
This then would have an error case if offset.nanos >= 1000 and offset.nanos % 1000 != 0
# ok!
In [11]: to_offset('1D').nanos % 1000
Out[11]: 0
# ok
In [12]: to_offset('10ms').nanos % 1000
Out[12]: 0
# ok
In [13]: to_offset('10us').nanos % 1000
Out[13]: 0
# ok (with the separate rounding)
In [14]: to_offset('10ns').nanos % 1000
Out[14]: 10
# NOT OK, this we would lose precision on! raise!
In [15]: to_offset('1010ns').nanos % 1000
Out[15]: 10
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.
Ah I see, this method makes more sense. Thanks!
I mistakenly thought .nanos
returned the numeric value after the time unit instead of the frequency in nanoseconds which is why I was using this method.
Raise warning for invalid frequencies
Altered the logic and now will raise a warning for an invalid frequency like |
thanks @mroeschke keep em coming! |
closes pandas-dev#15578 Author: Matt Roeschke <[email protected]> Closes pandas-dev#15588 from mroeschke/fix_15578 and squashes the following commits: af95baa [Matt Roeschke] BUG: Timestamp.round precision error for ns (pandas-dev#15578)
git diff upstream/master | flake8 --diff
Nice trick @jreback!
round()
patched forDatetimeIndex
andTimestamp
when rounding by ns.