8000 Convert DataFrame.rename to keyword only; simplify axis validation by WillAyd · Pull Request #29140 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

Convert DataFrame.rename to keyword only; simplify axis validation #29140

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 18 commits into from
Jan 9, 2020
Merged

Convert DataFrame.rename to keyword only; simplify axis validation #29140

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f09c6df
Worked around frame edges
WillAyd Oct 21, 2019
66acc90
Fixed frame tests
WillAyd Oct 21, 2019
177a440
Fixed Series tests
WillAyd Oct 21, 2019
b66e9e2
whatsnew
WillAyd Oct 21, 2019
2415e46
Cleaned up test cases
WillAyd Oct 21, 2019
296a9df
style updates
WillAyd Oct 21, 2019
6ba8518
Merge remote-tracking branch 'upstream/master' into fix-rename
WillAyd Jan 8, 2020
4128780
post merge fixes
WillAyd Jan 8, 2020
2776c7d
style fixups
WillAyd Jan 8, 2020
8e20fa4
fixups
WillAyd Jan 8, 2020
cca8eed
Added okexcept
WillAyd Jan 9, 2020
1f8e1cf
Black fixup
WillAyd Jan 9, 2020
4dc8bbe
mypy fixups
WillAyd Jan 9, 2020
1b261f8
flake8
WillAyd Jan 9, 2020
b5e54bd
Merge remote-tracking branch 'upstream/master' into fix-rename
WillAyd Jan 9, 2020
ab1ee2e
Comments addressed
WillAyd Jan 9, 2020
af0b7c5
minor fixups
WillAyd Jan 9, 2020
d5d812c
bound typevar to self
WillAyd Jan 9, 2020
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
Prev Previous commit
Next Next commit
Fixed Series tests
  • Loading branch information
@WillAyd
WillAyd committed Oct 21, 2019
commit 177a4409d819273ae316460f77207d8058fc9bbc
8 changes: 6 additions & 2 deletions pandas/core/series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
from pandas.core.strings import StringMethods
from pandas.core.tools.datetimes import to_datetime

from pandas._typing import Level
from pandas._typing import Axis, Level
import pandas.io.formats.format as fmt
import pandas.plotting

Expand Down Expand Up @@ -4086,6 +4086,7 @@ def rename(
Union[Hashable, Mapping[Hashable, Hashable], Callable[[Hashable], Hashable]]
] = None,
*,
axis: Optional[Axis] = None,
copy: bool = True,
inplace: bool = False,
level: Optional[Level] = None,
Expand All @@ -4104,6 +4105,8 @@ def rename(

Parameters
----------
axis : int or str
Unused. Accepted for compatability with DataFrame method only.
index : scalar, hashable sequence, dict-like or function, optional
Functions or dict-like are transformations to apply to
the index.
Expand All @@ -4125,6 +4128,7 @@ def rename(

See Also
--------
DataFrame.rename : Corresponding DataFrame method.
Series.rename_axis : Set the name of the axis.

Examples
Expand Down Expand Up @@ -4152,7 +4156,7 @@ def rename(
dtype: int64
"""
if callable(index) or is_dict_like(index):
return super().rename(index=index, copy=copy, inplace=inplace, level=level, errors=errors)
return super().rename(index, copy=copy, inplace=inplace, level=level, errors=errors)
else:
return self._set_name(index, inplace=inplace)

Expand Down
5 changes: 3 additions & 2 deletions pandas/tests/series/test_alter_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ def test_rename_axis_supported(self):
s = Series(range(5))
s.rename({}, axis=0)
s.rename({}, axis="index")
with pytest.raises(ValueError, match="No axis named 5"):
s.rename({}, axis=5)
# TODO: clean up shared index validation
Copy link
Member Author

Choose a reason for hiding this comment

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

Commented this out for now as I didn't think it was hugely critical and seemed somewhat orthoganal to the test, but can add back in if there is a huge objection.

Ideally though we would have a centralized place for index validation that doesn't mutate the arguments of the caller.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine to move this elsewhere. I do have a slight preference for not completely ignoring arguments that are there just for compatibility, but I don't know what our general rule is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea let me take another look I think should be able to keep this. Both self._get_axis_number and self._get_axis_name have some kind of validation in generic.

I don't know if there is one consistent way we do this right now, but want to march towards that

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 the preference should be that the axis parameter should be validated if specified on a Series for consistency with most Series methods that accept axis (e.g. fillna) and all DataFrame methods that accept axis.

Copy link
Member

Choose a reason for hiding this comment

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

Both self._get_axis_number and self._get_axis_name have some kind of validation in generic.

normally you can just pass axis to super and it'll get validated? in this case there is also the call to self._set_name (cause of #28920). so probably will need the call to self._get_axis_number.

# with pytest.raises(ValueError, match="No axis named 5"):
# s.rename({}, axis=5)

def test_set_name_attribute(self):
s = Series([1, 2, 3])
Expand Down
0