Skip to content

BUG: .rename* not treating Series as mapping #12626

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions doc/source/whatsnew/v0.18.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,8 @@ Bug Fixes
~~~~~~~~~

- Bug in ``value_counts`` when ``normalize=True`` and ``dropna=True`` where nulls still contributed to the normalized count (:issue:`12558`)


- Bug in ``Series.rename``, ``DataFrame.rename`` and ``DataFrame.rename_axis`` not treating ``Series`` as mappings to relabel (:issue:`12623`).


9 changes: 3 additions & 6 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,9 @@ def rename_axis(self, mapper, axis=0, copy=True, inplace=False):
1 2 5
2 3 6
"""
is_scalar_or_list = (
(not com.is_sequence(mapper) and not callable(mapper)) or
(com.is_list_like(mapper) and not com.is_dict_like(mapper))
)

if is_scalar_or_list:
non_mapper = lib.isscalar(mapper) or (com.is_list_like(mapper) and not
com.is_dict_like(mapper))
if non_mapper:
return self._set_axis_name(mapper, axis=axis)
else:
axis = self._get_axis_name(axis)
Expand Down
9 changes: 3 additions & 6 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import types
import warnings
from collections import MutableMapping

from numpy import nan, ndarray
import numpy as np
Expand Down Expand Up @@ -2331,11 +2330,9 @@ def align(self, other, join='outer', axis=None, level=None, copy=True,

@Appender(generic._shared_docs['rename'] % _shared_doc_kwargs)
def rename(self, index=None, **kwargs):
is_scalar_or_list = (
(not com.is_sequence(index) and not callable(index)) or
(com.is_list_like(index) and not isinstance(index, MutableMapping))
)
if is_scalar_or_list:
non_mapping = lib.isscalar(index) or (com.is_list_like(index) and
not com.is_dict_like(index))
if non_mapping:
return self._set_name(index, inplace=kwargs.get('inplace'))
return super(Series, self).rename(index=index, **kwargs)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I think lib.isscalar(...) or com.is_list_like(...) is more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

com.is_list_like(dict) is True, and I need to treat dicts differently than lists.

But if we're going to be raising on Series.rename(list) anway, I won't have to worry about lists at all...

Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/series/test_alter_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ def test_rename(self):
renamed = renamer.rename({})
self.assertEqual(renamed.index.name, renamer.index.name)

def test_rename_by_series(self):
s = Series(range(5), name='foo')
renamer = Series({1: 10, 2: 20})
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe tests both dict and Series (for the renamer), and anything else that could be there (e.g. callable)? you prob already have these tests......

d = {1: 10, 2: 20}
for value in [ d, Series(d) ]:
     ....

for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should all covered by test_generic now. Initially I didn't understand how test_generic was setup, so I added the test here. I think these tests are all redundant, and could remove them this one and test_rename above, but I always worry about deleting tests :)

Copy link
Contributor

Choose a reason for hiding this comment

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

cool np. yeah. hard to stop the propogation of tests :)

result = s.rename(renamer)
expected = Series(range(5), index=[0, 10, 20, 3, 4], name='foo')
tm.assert_series_equal(result, expected)

def test_rename_set_name(self):
s = Series(range(4), index=list('abcd'))
for name in ['foo', ['foo'], ('foo',)]:
Expand Down
50 changes: 41 additions & 9 deletions pandas/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,53 @@ def _compare(self, result, expected):
def test_rename(self):

# single axis
idx = list('ABCD')
# relabeling values passed into self.rename
args = [
str.lower,
{x: x.lower() for x in idx},
Series({x: x.lower() for x in idx}),
]

for axis in self._axes():
kwargs = {axis: list('ABCD')}
kwargs = {axis: idx}
obj = self._construct(4, **kwargs)

# no values passed
# self.assertRaises(Exception, o.rename(str.lower))

# rename a single axis
result = obj.rename(**{axis: str.lower})
expected = obj.copy()
setattr(expected, axis, list('abcd'))
self._compare(result, expected)
for arg in args:
# rename a single axis
result = obj.rename(**{axis: arg})
expected = obj.copy()
setattr(expected, axis, list('abcd'))
self._compare(result, expected)

# multiple axes at once

def test_rename_axis(self):
idx = list('ABCD')
# relabeling values passed into self.rename
args = [
str.lower,
{x: x.lower() for x in idx},
Series({x: x.lower() for x in idx}),
]

for axis in self._axes():
kwargs = {axis: idx}
obj = self._construct(4, **kwargs)

for arg in args:
# rename a single axis
result = obj.rename_axis(arg, axis=axis)
expected = obj.copy()
setattr(expected, axis, list('abcd'))
self._compare(result, expected)
# scalar values
for arg in ['foo', None]:
result = obj.rename_axis(arg, axis=axis)
expected = obj.copy()
getattr(expected, axis).name = arg
self._compare(result, expected)

def test_get_numeric_data(self):

n = 4
Expand Down