-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
PERF: add shortcut to Timedelta constructor #31070
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
Changes from 7 commits
eac6244
8c6d2b8
25b7c46
a9ff18f
58937f0
3c4af45
9df1fae
79b898f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -950,3 +950,10 @@ def test_datetimeindex_constructor_misc(self): | |
) | ||
assert len(idx1) == len(idx2) | ||
assert idx1.freq == idx2.freq | ||
|
||
|
||
def test_timedelta_constructor_identity(): | ||
# Test for #30543 | ||
expected = pd.Timedelta(np.timedelta64(1, "s")) | ||
result = pd.Timedelta(expected) | ||
assert result is expected | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this expectation - we don't do this for Timestamps so not sure should do it for Timedeltas as well but maybe @jschendel has thoughts There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a test for the constructor identity too, in the Timestamp shortcut PR #30676. Sorry for posting the Timedelta one before that one gets merged. It's just that the discussion was mostly done there, so I thought I'd get on the Timedelta one too. |
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.
Since we are here, let's add a benchmark for making Timedelta out of Timedelta, and I think we should also tweak existing tests. Currently, we benchmark both our method and
np.timedelta64
anddatetime.timedelta
at the same time. We should move the initialization intosetup
, in my opinion. It's a bit nitpicky, of course, since bothnp.timedelta64
anddatetime.timedelta
are unlikely to change speed in the future, but still.