-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
API/DEPR: Change default skipna behaviour + deprecate numeric_only in Categorical.min and max #27929
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
API/DEPR: Change default skipna behaviour + deprecate numeric_only in Categorical.min and max #27929
Changes from 7 commits
9fab462
4bfe686
e4d5c38
765e506
300a862
095e6da
f094635
7d73163
f9d9bc5
6a184c8
ac89bcd
9691c44
d281650
23ffd16
260201c
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 |
---|---|---|
|
@@ -2193,7 +2193,8 @@ def _reduce(self, name, axis=0, **kwargs): | |
raise TypeError(msg.format(op=name)) | ||
return func(**kwargs) | ||
|
||
def min(self, numeric_only=None, **kwargs): | ||
@deprecate_kwarg(old_arg_name="numeric_only", new_arg_name="skipna") | ||
def min(self, skipna=None, **kwargs): | ||
""" | ||
The minimum value of the object. | ||
|
||
|
@@ -2209,17 +2210,24 @@ def min(self, numeric_only=None, **kwargs): | |
min : the minimum of this `Categorical` | ||
""" | ||
self.check_for_ordered("min") | ||
if numeric_only: | ||
good = self._codes != -1 | ||
pointer = self._codes[good].min(**kwargs) | ||
good = self._codes != -1 | ||
if not good.all(): | ||
if skipna: | ||
pointer = self._codes[good].min(**kwargs) | ||
else: | ||
if skipna is None: | ||
msg = ( | ||
"The default value of skipna will be changed to " | ||
"True in the future version." | ||
) | ||
warn(msg, FutureWarning, stacklevel=2) | ||
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. You can remove this |
||
return np.nan | ||
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 is not correct for i8 types, which should be pd.NaT. how to fix this? 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. We could check the |
||
else: | ||
pointer = self._codes.min(**kwargs) | ||
if pointer == -1: | ||
return np.nan | ||
else: | ||
return self.categories[pointer] | ||
return self.categories[pointer] | ||
|
||
def max(self, numeric_only=None, **kwargs): | ||
@deprecate_kwarg(old_arg_name="numeric_only", new_arg_name="skipna") | ||
def max(self, skipna=None, **kwargs): | ||
""" | ||
The maximum value of the object. | ||
|
||
|
@@ -2235,15 +2243,21 @@ def max(self, numeric_only=None, **kwargs): | |
max : the maximum of this `Categorical` | ||
""" | ||
self.check_for_ordered("max") | ||
if numeric_only: | ||
good = self._codes != -1 | ||
pointer = self._codes[good].max(**kwargs) | ||
good = self._codes != -1 | ||
if not good.all(): | ||
if skipna: | ||
pointer = self._codes[good].max(**kwargs) | ||
else: | ||
if skipna is None: | ||
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. same here |
||
msg = ( | ||
"The default value of skipna will be changed to " | ||
"True in the future version." | ||
) | ||
warn(msg, FutureWarning, stacklevel=2) | ||
return np.nan | ||
else: | ||
pointer = self._codes.max(**kwargs) | ||
if pointer == -1: | ||
return np.nan | ||
else: | ||
return self.categories[pointer] | ||
return self.categories[pointer] | ||
|
||
def mode(self, dropna=True): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,31 +35,45 @@ def test_min_max(self): | |
assert _min == "d" | ||
assert _max == "a" | ||
|
||
@pytest.mark.parametrize("skipna", [True, False]) | ||
def test_min_max_with_nan(self, skipna): | ||
# GH 25303 | ||
cat = Categorical( | ||
[np.nan, "b", "c", np.nan], categories=["d", "c", "b", "a"], ordered=True | ||
) | ||
_min = cat.min() | ||
_max = cat.max() | ||
assert np.isnan(_min) | ||
assert _max == "b" | ||
_min = cat.min(skipna=skipna) | ||
_max = cat.max(skipna=skipna) | ||
|
||
_min = cat.min(numeric_only=True) | ||
assert _min == "c" | ||
_max = cat.max(numeric_only=True) | ||
assert _max == "b" | ||
if skipna is False: | ||
assert np.isnan(_min) | ||
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. use isna/notna 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.
|
||
assert np.isnan(_max) | ||
else: | ||
assert _min == "c" | ||
assert _max == "b" | ||
|
||
cat = Categorical( | ||
[np.nan, 1, 2, np.nan], categories=[5, 4, 3, 2, 1], ordered=True | ||
) | ||
_min = cat.min() | ||
_max = cat.max() | ||
assert np.isnan(_min) | ||
assert _max == 1 | ||
|
||
_min = cat.min(numeric_only=True) | ||
assert _min == 2 | ||
_max = cat.max(numeric_only=True) | ||
assert _max == 1 | ||
_min = cat.min(skipna=skipna) | ||
_max = cat.max(skipna=skipna) | ||
|
||
if skipna is False: | ||
assert np.isnan(_min) | ||
assert np.isnan(_max) | ||
else: | ||
assert _min == 2 | ||
assert _max == 1 | ||
|
||
@pytest.mark.parametrize("method", ["min", "max"]) | ||
def test_deprecate_numeric_only_min_max(self, method): | ||
# GH 25303 | ||
cat = Categorical( | ||
[np.nan, 1, 2, np.nan], categories=[5, 4, 3, 2, 1], ordered=True | ||
) | ||
with tm.assert_produces_warning( | ||
expected_warning=FutureWarning, check_stacklevel=False | ||
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. Does it work without |
||
): | ||
getattr(cat, method)(numeric_only=True) | ||
|
||
@pytest.mark.parametrize( | ||
"values,categories,exp_mode", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1043,7 +1043,7 @@ def test_min_max(self): | |
) | ||
_min = cat.min() | ||
_max = cat.max() | ||
assert np.isnan(_min) | ||
assert _min == "c" | ||
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. Why idd this change? 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. Please refer to the following comment. |
||
assert _max == "b" | ||
|
||
cat = Series( | ||
|
@@ -1053,30 +1053,24 @@ def test_min_max(self): | |
) | ||
_min = cat.min() | ||
_max = cat.max() | ||
assert np.isnan(_min) | ||
assert _min == 2 | ||
assert _max == 1 | ||
|
||
def test_min_max_numeric_only(self): | ||
# TODO deprecate numeric_only argument for Categorical and use | ||
# skipna as well, see GH25303 | ||
@pytest.mark.parametrize("skipna", [True, False]) | ||
def test_min_max_skipna(self, skipna): | ||
# GH 25303 | ||
cat = Series( | ||
Categorical(["a", "b", np.nan, "a"], categories=["b", "a"], ordered=True) | ||
) | ||
_min = cat.min(skipna=skipna) | ||
_max = cat.max(skipna=skipna) | ||
|
||
_min = cat.min() | ||
_max = cat.max() | ||
assert np.isnan(_min) | ||
assert _max == "a" | ||
|
||
_min = cat.min(numeric_only=True) | ||
_max = cat.max(numeric_only=True) | ||
assert _min == "b" | ||
assert _max == "a" | ||
|
||
_min = cat.min(numeric_only=False) | ||
_max = cat.max(numeric_only=False) | ||
assert np.isnan(_min) | ||
assert _max == "a" | ||
if skipna is True: | ||
assert _min == "b" | ||
assert _max == "a" | ||
else: | ||
assert np.isnan(_min) | ||
assert np.isnan(_max) | ||
|
||
|
||
class TestSeriesMode: | ||
|
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 can be skipna=True ?