Skip to content

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Dec 2, 2019
Merged

API/DEPR: Change default skipna behaviour + deprecate numeric_only in Categorical.min and max #27929

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9fab462
Deprecate numeric_only parameter in Categorical.min and max
makbigc Aug 15, 2019
4bfe686
Amend after 1st review
makbigc Aug 16, 2019
e4d5c38
Parametrize test_min_max_skipna in reductions/test_reductions.py
makbigc Aug 16, 2019
765e506
merge for update
makbigc Oct 30, 2019
300a862
Set skipna=None by default and add future warning
makbigc Nov 1, 2019
095e6da
Fix black issue and the test error
makbigc Nov 1, 2019
f094635
merge for update
makbigc Nov 21, 2019
7d73163
Set skipna=True and add section in API breaking
makbigc Nov 21, 2019
f9d9bc5
merge for update
makbigc Nov 23, 2019
6a184c8
Modify after review
makbigc Nov 23, 2019
ac89bcd
merge for solving conflict
makbigc Nov 26, 2019
9691c44
merge for update
makbigc Nov 27, 2019
d281650
merge for update
makbigc Dec 1, 2019
23ffd16
Remove kwarg in categorical.min and max
makbigc Dec 2, 2019
260201c
Change wording in v1.0.0.rst
makbigc Dec 2, 2019
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix black issue and the test error
  • Loading branch information
@makbigc
makbigc committed Nov 1, 2019
commit 095e6da6613fce89b39d3b5d095b87f465bbac02
18 changes: 12 additions & 6 deletions pandas/core/arrays/categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2216,9 +2216,12 @@ def min(self, skipna=None, **kwargs):
if skipna:
pointer = self._codes[good].min(**kwargs)
else:
msg = ("The default value of skipna will be changed to "
"True in the future version.")
warn(msg, FutureWarning, stacklevel=2)
if skipna is None:
msg = (
"The default value of skipna will be changed to "
"True in the future version."
)
warn(msg, FutureWarning, stacklevel=2)
Copy link
Member

@jorisvandenbossche jorisvandenbossche Nov 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this if skipna is None block with the warning I think?
return np.nan
Copy link
Contributor

@jreback jreback Nov 25, 2019

Choose a reason for hiding this comment

The 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?
Copy link
Member

@jorisvandenbossche jorisvandenbossche Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could check the categories.dtype.na_value if it exists. But since this is the current behaviour, it's not critical to fix in this PR I think.
else:
pointer = self._codes.min(**kwargs)
Expand Down Expand Up @@ -2246,9 +2249,12 @@ def max(self, skipna=None, **kwargs):
if skipna:
pointer = self._codes[good].max(**kwargs)
else:
msg = ("The default value of skipna will be changed to "
"True in the future version.")
warn(msg, FutureWarning, stacklevel=2)
if skipna is None:
Copy link
Member

@jorisvandenbossche jorisvandenbossche Nov 23, 2019

Choose a reason for hiding this comment

The 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)
Expand Down
5 changes: 3 additions & 2 deletions pandas/tests/arrays/categorical/test_analytics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ def test_deprecate_numeric_only_min_max(self, method):
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):
with tm.assert_produces_warning(
expected_warning=FutureWarning, check_stacklevel=False
Copy link
Member

@jorisvandenbossche jorisvandenbossche Nov 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work without check_stacklevel=False ?
):
getattr(cat, method)(numeric_only=True)

@pytest.mark.parametrize(
Expand Down