-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Fix pandas.Timedelta range #12728
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 pandas.Timedelta range #12728
Conversation
|
||
# Resolution is in nanoseconds | ||
# (2**63)/(1*24*60*60*(10**9)) = 106751.991167 | ||
Timedelta.min = Timedelta(-106751.991167, 'D') |
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.
Can we specify this in ns
instead of fractional days, which might have rounding errors? e.g., Timedelta(np.iinfo(np.int64).min, 'ns')
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.
Actually, looks like you need min + 1
since min
corresponds to NaT
:
In [6]: Timedelta(np.iinfo(np.int64).min, 'ns')
Out[6]: NaT
In [7]: Timedelta(np.iinfo(np.int64).min + 1, 'ns')
Out[7]: Timedelta('-106752 days +00:12:43.145224')
In [8]: Timedelta(np.iinfo(np.int64).max, 'ns')
Out[8]: Timedelta('106751 days 23:47:16.854775')
af5fdff
to
94b886b
Compare
This concerns the 2nd commit, in this PR @jreback, can you confirm that some of the tests at rely on buggy output. Specifically, that rng = timedelta_range('1 day 10:11:12', freq='h', periods=500)
s = Series(np.arange(len(rng)), index=rng)
result = s['5 day':'6 day']
expected = s.iloc[86:110]
assert_series_equal(result, expected) If this is a second bug fix, then I will address the changes documentation. Maybe there is even a better way other than peaking at the indices and creating the |
@@ -1680,7 +1680,7 @@ def test_partial_slice(self): | |||
s = Series(np.arange(len(rng)), index=rng) | |||
|
|||
result = s['5 day':'6 day'] | |||
expected = s.iloc[86:134] | |||
expected = s.iloc[86:110] |
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.
why are you changing this?
94b886b
to
99f3ba9
Compare
@@ -109,6 +109,31 @@ The ``unit`` keyword argument specifies the unit of the Timedelta: | |||
to_timedelta(np.arange(5), unit='s') | |||
to_timedelta(np.arange(5), unit='d') | |||
|
|||
timdelta limits |
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.
Timedelta Limits
pls add a whatsnew note, point to the new docs (with a thanks. |
*Problem* Pandas Timedelta derives from `datetime.timedelta` and increases the resolution of the timedeltas to nanoseconds. As such Pandas.Timedelta has a smaller range of values. *Solution* This change modifies the advertised `min` and `max` timedeltas.
99f3ba9
to
2b78e5a
Compare
thanks @has2k1 |
git diff upstream/master | flake8 --diff
Problem
Pandas Timedelta derives from
datetime.timedelta
and increasethe resolution of the timedeltas. As such the Pandas.Timedelta
object can only have a smaller range of values.
Solution
This change modifies the properties that report
the range and resolution to reflect Pandas capabilities.
Reference
https://github.com/python/cpython/blob/8d1d7e6816753248768e4cc1c0370221814e9cf1/Lib/datetime.py#L651-L654