-
-
Notifications
You must be signed in to change notification settings - Fork 18.7k
Implement reductions from #24024 #24484
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
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.
maybe out th treats in the new pandas/tests/reductions
Index.max : Return the maximum value in an Index. | ||
Series.max : Return the maximum value in a Series. | ||
""" | ||
# TODO: skipna is broken with max. |
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.
this was just fixed?
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.
I thought so too, but no. I expect it won't be too tough to fix nanops to apply the DTI fix to DTA, will do so in an upcoming pass after the 24024-specific parts are de-duplicated
|
||
class TestReductions(object): | ||
|
||
@pytest.mark.parametrize("tz", [None, "US/Central"]) |
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.
in test_reductions?
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.
before long, yah. For now i want to make rebasing 24024 easy on tom
ping on green. |
Codecov Report
@@ Coverage Diff @@
## master #24484 +/- ##
==========================================
+ Coverage 92.3% 92.31% +<.01%
==========================================
Files 166 166
Lines 52386 52412 +26
==========================================
+ Hits 48357 48382 +25
- Misses 4029 4030 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24484 +/- ##
===========================================
- Coverage 92.31% 43.03% -49.28%
===========================================
Files 166 166
Lines 52335 52361 +26
===========================================
- Hits 48313 22535 -25778
- Misses 4022 29826 +25804
Continue to review full report at Codecov.
|
this will need rebase following #24476. |
Ping |
|
||
def min(self, axis=None, skipna=True, *args, **kwargs): | ||
""" | ||
Return the minimum value of the Array or minimum along |
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 these meant in inherit docstrings? these should have Parameters if not
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.
these are copied from the DatetimeIndexOps versions, with "Index" changes to "Array" and Index.min/Index.max added to the See Also sections. These will be templated/shared before long hopefully
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.
ok can u make a follow up issue for this
min/max signatures+docstrings are changed from #24024, instead match those of DatetimeIndexOpsMixin (with appropriate docstring edits)
The min/max implementations are unchanged from 24024. In an upcoming pass I'll see if we can pawn any more of it off to nanops.
Tests are verbatim from 24024.