Skip to content
Merged
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ Deprecations
- Deprecated :attr:`Rolling.is_datetimelike` (:issue:`38963`)
- Deprecated :meth:`core.window.ewm.ExponentialMovingWindow.vol` (:issue:`39220`)
- Using ``.astype`` to convert between ``datetime64[ns]`` dtype and :class:`DatetimeTZDtype` is deprecated and will raise in a future version, use ``obj.tz_localize`` or ``obj.dt.tz_localize`` instead (:issue:`38622`)
- The ``inplace`` parameter of :meth:`Categorical.remove_categories` is deprecated and will be removed in a future version (:issue:`37643`)
-

.. ---------------------------------------------------------------------------
Expand Down
27 changes: 23 additions & 4 deletions pandas/core/arrays/categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
Union,
cast,
)
from warnings import warn
from warnings import catch_warnings, simplefilter, warn

import numpy as np

Expand Down Expand Up @@ -1057,7 +1057,7 @@ def add_categories(self, new_categories, inplace=False):
if not inplace:
return cat

def remove_categories(self, removals, inplace=False):
def remove_categories(self, removals, inplace=no_default):
"""
Remove the specified categories.

Expand All @@ -1072,6 +1072,8 @@ def remove_categories(self, removals, inplace=False):
Whether or not to remove the categories inplace or return a copy of
this categorical with removed categories.

.. deprecated:: 1.2.0
Copy link
Member

Choose a reason for hiding this comment

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

1.3.0 now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, thanks. I updated the version here and fixed conflicts with the master


Returns
-------
cat : Categorical or None
Expand All @@ -1090,6 +1092,17 @@ def remove_categories(self, removals, inplace=False):
remove_unused_categories : Remove categories which are not used.
set_categories : Set the categories to the specified ones.
"""
if inplace is not no_default:
warn(
"The `inplace` parameter in pandas.Categorical."
"remove_categories is deprecated and "
"will be removed in a future version.",
Copy link
Member

Choose a reason for hiding this comment

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

usually we would say "do X instead", but since the idea is to never change the dtype inplace, might be worth adding a short sentence like "Removing unused categories will always return a new Categorical object"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I updated the warning message.

FutureWarning,
stacklevel=2,
)
else:
inplace = False

inplace = validate_bool_kwarg(inplace, "inplace")
if not is_list_like(removals):
removals = [removals]
Expand Down Expand Up @@ -2324,14 +2337,20 @@ def replace(self, to_replace, value, inplace: bool = False):
continue
if replace_value in cat.categories:
if isna(new_value):
cat.remove_categories(replace_value, inplace=True)
with catch_warnings():
simplefilter("ignore")
cat.remove_categories(replace_value, inplace=True)
Copy link
Member

Choose a reason for hiding this comment

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

i think eventually we'll need to push this up and deprecate inplace here in Categorical.replace, but that can be done in a separate step

continue

categories = cat.categories.tolist()
index = categories.index(replace_value)

if new_value in cat.categories:
value_index = categories.index(new_value)
cat._codes[cat._codes == index] = value_index
cat.remove_categories(replace_value, inplace=True)
with catch_warnings():
simplefilter("ignore")
cat.remove_categories(replace_value, inplace=True)
else:
categories[index] = new_value
cat.rename_categories(categories, inplace=True)
Expand Down
4 changes: 3 additions & 1 deletion pandas/tests/arrays/categorical/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,9 @@ def test_validate_inplace_raises(self, value):
cat.add_categories(new_categories=["D", "E", "F"], inplace=value)

with pytest.raises(ValueError, match=msg):
cat.remove_categories(removals=["D", "E", "F"], inplace=value)
with tm.assert_produces_warning(FutureWarning):
# issue #37643 inplace kwarg deprecated
Copy link
Member

Choose a reason for hiding this comment

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

nit: I'd drop this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was asked to add this comment in my previous pull request #37918, which is related to #37643, so I did the same here

Copy link
Member

Choose a reason for hiding this comment

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

yah i find these helpful

cat.remove_categories(removals=["D", "E", "F"], inplace=value)

with pytest.raises(ValueError, match=msg):
with tm.assert_produces_warning(FutureWarning):
Expand Down
5 changes: 4 additions & 1 deletion pandas/tests/arrays/categorical/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,10 @@ def test_remove_categories(self):
tm.assert_categorical_equal(res, new)

# inplace == True
res = cat.remove_categories("c", inplace=True)
with tm.assert_produces_warning(FutureWarning):
# issue #37643 inplace kwarg deprecated
Copy link
Member

Choose a reason for hiding this comment

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

and this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

res = cat.remove_categories("c", inplace=True)

tm.assert_categorical_equal(cat, new)
assert res is None

Expand Down