Skip to content
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

Rename categories with Series #17982

Merged
merged 9 commits into from
Oct 26, 2017
Merged

Rename categories with Series #17982

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b551b28
PERF/API: Treat series as array-like for rename_categories
TomAugspurger Oct 25, 2017
19af62a
Redo docstring
TomAugspurger Oct 25, 2017
f659e78
Use list-like
TomAugspurger Oct 25, 2017
6460bc5
Warn
TomAugspurger Oct 26, 2017
506f9ba
Fix doc indent
TomAugspurger Oct 26, 2017
4f6a1dd
Doc cleanup
TomAugspurger Oct 26, 2017
cbcc723
More doc cleanup
TomAugspurger Oct 26, 2017
aec380d
Fix API reference
TomAugspurger Oct 26, 2017
0e086ca
Typos
TomAugspurger Oct 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions pandas/core/categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,11 +866,6 @@ def set_categories(self, new_categories, ordered=None, rename=False,
def rename_categories(self, new_categories, inplace=False):
""" Renames categories.

The new categories can be either a list-like dict-like object.
If it is list-like, all items must be unique and the number of items
in the new categories must be the same as the number of items in the
old categories.

Raises
------
ValueError
Expand All @@ -879,8 +874,22 @@ def rename_categories(self, new_categories, inplace=False):

Parameters
----------
new_categories : Index-like or dict-like (>=0.21.0)
The renamed categories.
new_categories : list-like or dict-like
The categories end up with
Copy link
Member

Choose a reason for hiding this comment

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

this seems like an incomplete sentence ?


.. versionchanged:: 0.21.0

new_categories may now also be dict-like, in which case it
specifies a mapping from old-categories to new.

If it is list-like, all items must be unique and the number of
items in the new categories must match the existing number of
categories.

If dict-like, categories not contained in the mapping are passed
through. Note that ``Series`` are considered list-like in this
context.

inplace : boolean (default: False)
Whether or not to rename the categories inplace or return a copy of
this categorical with renamed categories.
Expand All @@ -896,11 +905,33 @@ def rename_categories(self, new_categories, inplace=False):
remove_categories
remove_unused_categories
set_categories

Examples
--------
>>> c = Categorical(['a', 'a', 'b'])
>>> c.rename_categories([0, 1])
[0, 0, 1]
Categories (2, int64): [0, 1]

For dict-like ``new_categories``, extra keys are ignored and
categories not in the dictionary are passed through

>>> c.rename_categories({'a': 'A', 'c': 'C'})
[A, A, b]
Categories (2, object): [A, b]

Series are considered list-like here, so the *values* are used
instead of the *index*
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually want this behaviour?
Eg for Series.rename, a Series is seen as a dict-like ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. It’ll be a backwards incompatible change if we don’t treat Series as arrays so I think we should at least do this for now, maybe with a warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I think this is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback which part to you agree with? Warning that it'll change to dict-like in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

no I agree the current behavior is correct. we handle list-like the same. no warning is needed as this is expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, looking at this again.

a Series should be just like a dict. This is a perf issue yes?
do this. (once Index.map works we could simplify a bit)

In [12]: cat = pd.Categorical(['a', 'b', 'c', 'd'])
    ...: res = cat.rename_categories(pd.Series({'a': 4, 'b': 3, 'c': 2, 'd': 1}))
    ...: 
    ...: 

In [13]: cat
Out[13]: 
[a, b, c, d]
Categories (4, object): [a, b, c, d]

In [14]: res
Out[14]: 
[4, 3, 2, 1]
Categories (4, int64): [4, 3, 2, 1]

In [17]: pd.Series(cat.categories).map({'a': 4, 'b': 3, 'c': 2, 'd': 1}).values
Out[17]: array([4, 3, 2, 1])

In [19]: pd.Series(cat.categories).map({'a': 4, 'b': 3, 'c': 2, 'd': 1}).values
Out[19]: array([4, 3, 2, 1])

Copy link
Contributor

Choose a reason for hiding this comment

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

please revert the warning

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain?
If we go for "Series -> dict-like" behaviour, this is a breaking change, and we need to use a warning for that.


>>> c.rename_categories(pd.Series([0, 1], index=['a', 'b']))
[0, 0, 1]
Categories (2, int64): [0, 1]
"""
inplace = validate_bool_kwarg(inplace, 'inplace')
cat = self if inplace else self.copy()

if is_dict_like(new_categories):
if (is_dict_like(new_categories) and
not isinstance(new_categories, ABCSeries)):
cat.categories = [new_categories.get(item, item)
Copy link
Contributor

Choose a reason for hiding this comment

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

just use map

for item in cat.categories]
else:
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/test_categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,13 @@ def test_rename_categories(self):
with pytest.raises(ValueError):
cat.rename_categories([1, 2])

def test_rename_categories_series(self):
# https://github.com/pandas-dev/pandas/issues/17981
result = pd.Categorical(['a', 'b']).rename_categories(
pd.Series([0, 1]))
expected = pd.Categorical([0, 1])
tm.assert_categorical_equal(result, expected)

def test_rename_categories_dict(self):
# GH 17336
cat = pd.Categorical(['a', 'b', 'c', 'd'])
Expand Down