-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Changes from 2 commits
a1bdb34
45761d5
e3fb0ef
244d37b
56bcc80
bf99607
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 |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
|
||
from pandas.core.algorithms import checked_add_with_arr, take, unique1d | ||
import pandas.core.common as com | ||
from pandas.core import nanops | ||
|
||
from pandas.tseries import frequencies | ||
from pandas.tseries.offsets import DateOffset, Tick | ||
|
@@ -1381,6 +1382,71 @@ def _ensure_localized(self, arg, ambiguous='raise', nonexistent='raise', | |
) | ||
return arg | ||
|
||
# -------------------------------------------------------------- | ||
# Reductions | ||
|
||
def _reduce(self, name, axis=0, skipna=True, **kwargs): | ||
op = getattr(self, name, None) | ||
if op: | ||
return op(axis=axis, skipna=skipna, **kwargs) | ||
else: | ||
raise TypeError("cannot perform {name} with type {dtype}" | ||
.format(name=name, dtype=self.dtype)) | ||
# TODO: use super(DatetimeLikeArrayMixin, self)._reduce | ||
# after we subclass ExtensionArray | ||
|
||
def min(self, axis=None, skipna=True, *args, **kwargs): | ||
""" | ||
Return the minimum value of the Array or minimum along | ||
an axis. | ||
|
||
See Also | ||
-------- | ||
numpy.ndarray.min | ||
Index.min : Return the minimum value in an Index. | ||
Series.min : Return the minimum value in a Series. | ||
""" | ||
nv.validate_min(args, kwargs) | ||
nv.validate_minmax_axis(axis) | ||
|
||
result = nanops.nanmin(self.asi8, skipna=skipna, mask=self.isna()) | ||
if isna(result): | ||
# Period._from_ordinal does not handle np.nan gracefully | ||
return NaT | ||
return self._box_func(result) | ||
|
||
def max(self, axis=None, skipna=True, *args, **kwargs): | ||
""" | ||
Return the maximum value of the Array or maximum along | ||
an axis. | ||
|
||
See Also | ||
-------- | ||
numpy.ndarray.max | ||
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 commentThe 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 commentThe 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 |
||
# See https://github.com/pandas-dev/pandas/issues/24265 | ||
nv.validate_max(args, kwargs) | ||
nv.validate_minmax_axis(axis) | ||
|
||
mask = self.isna() | ||
if skipna: | ||
values = self[~mask].asi8 | ||
elif mask.any(): | ||
return NaT | ||
else: | ||
values = self.asi8 | ||
|
||
if not len(values): | ||
# short-circut for empty max / min | ||
return NaT | ||
|
||
result = nanops.nanmax(values, skipna=skipna) | ||
# Don't have to worry about NA `result`, since no NA went in. | ||
return self._box_func(result) | ||
|
||
|
||
DatetimeLikeArrayMixin._add_comparison_ops() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,3 +126,41 @@ def test_tz_dtype_matches(self): | |
result, _, _ = sequence_to_dt64ns( | ||
arr, dtype=DatetimeTZDtype(tz="US/Central")) | ||
tm.assert_numpy_array_equal(arr._data, result) | ||
|
||
|
||
class TestReductions(object): | ||
|
||
@pytest.mark.parametrize("tz", [None, "US/Central"]) | ||
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. in test_reductions? 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. before long, yah. For now i want to make rebasing 24024 easy on tom |
||
def test_min_max(self, tz): | ||
arr = DatetimeArray._from_sequence([ | ||
'2000-01-03', | ||
'2000-01-03', | ||
'NaT', | ||
'2000-01-02', | ||
'2000-01-05', | ||
'2000-01-04', | ||
], tz=tz) | ||
|
||
result = arr.min() | ||
expected = pd.Timestamp('2000-01-02', tz=tz) | ||
assert result == expected | ||
|
||
result = arr.max() | ||
expected = pd.Timestamp('2000-01-05', tz=tz) | ||
assert result == expected | ||
|
||
result = arr.min(skipna=False) | ||
assert result is pd.NaT | ||
|
||
result = arr.max(skipna=False) | ||
assert result is pd.NaT | ||
|
||
@pytest.mark.parametrize("tz", [None, "US/Central"]) | ||
@pytest.mark.parametrize('skipna', [True, False]) | ||
def test_min_max_empty(self, skipna, tz): | ||
arr = DatetimeArray._from_sequence([], tz=tz) | ||
result = arr.min(skipna=skipna) | ||
assert result is pd.NaT | ||
|
||
result = arr.max(skipna=skipna) | ||
assert result is pd.NaT |
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