-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
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.
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
Changes from all commits
f09c6df
66acc90
177a440
b66e9e2
2415e46
296a9df
6ba8518
4128780
2776c7d
8e20fa4
cca8eed
1f8e1cf
4dc8bbe
1b261f8
b5e54bd
ab1ee2e
af0b7c5
d5d812c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ | |
from pandas._config import get_option | ||
|
||
from pandas._libs import algos as libalgos, lib | ||
from pandas._typing import Axes, Dtype, FilePathOrBuffer | ||
from pandas._typing import Axes, Axis, Dtype, FilePathOrBuffer, Level, Renamer | ||
from pandas.compat import PY37 | ||
from pandas.compat._optional import import_optional_dependency | ||
from pandas.compat.numpy import function as nv | ||
|
@@ -3986,7 +3986,19 @@ def drop( | |
"mapper", | ||
[("copy", True), ("inplace", False), ("level", None), ("errors", "ignore")], | ||
) | ||
def rename(self, *args, **kwargs): | ||
def rename( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change here was to be explicit about what is accepted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to being explicit. The only reason we had *args, **kwargs in the first place was to support the ambiguous case. |
||
self, | ||
mapper: Optional[Renamer] = None, | ||
*, | ||
index: Optional[Renamer] = None, | ||
columns: Optional[Renamer] = None, | ||
axis: Optional[Axis] = None, | ||
copy: bool = True, | ||
inplace: bool = False, | ||
level: Optional[Level] = None, | ||
errors: str = "ignore", | ||
) -> Optional["DataFrame"]: | ||
|
||
""" | ||
Alter axes labels. | ||
|
||
|
@@ -4095,12 +4107,16 @@ def rename(self, *args, **kwargs): | |
2 2 5 | ||
4 3 6 | ||
""" | ||
axes = validate_axis_style_args(self, args, kwargs, "mapper", "rename") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Side note - there is only one other use now of this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there anything blocking a similar PR for
Is this referring to DataFrame.reindex, or validate_axis_style_args? Ideally we can completely remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't look before because I was trying to keep this focused, but it might not be too much extra effort. I'll take a look and if it gets out of hand let you know |
||
kwargs.update(axes) | ||
# Pop these, since the values are in `kwargs` under different names | ||
kwargs.pop("axis", None) | ||
kwargs.pop("mapper", None) | ||
return super().rename(**kwargs) | ||
return super().rename( | ||
mapper=mapper, | ||
index=index, | ||
columns=columns, | ||
axis=axis, | ||
copy=copy, | ||
inplace=inplace, | ||
level=level, | ||
errors=errors, | ||
) | ||
|
||
@Substitution(**_shared_doc_kwargs) | ||
@Appender(NDFrame.fillna.__doc__) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,15 @@ | |
from pandas._config import config | ||
|
||
from pandas._libs import Timestamp, iNaT, lib, properties | ||
from pandas._typing import Dtype, FilePathOrBuffer, FrameOrSeries, JSONSerializable | ||
from pandas._typing import ( | ||
Axis, | ||
Dtype, | ||
FilePathOrBuffer, | ||
FrameOrSeries, | ||
JSONSerializable, | ||
Level, | ||
Renamer, | ||
) | ||
from pandas.compat import set_function_name | ||
from pandas.compat._optional import import_optional_dependency | ||
from pandas.compat.numpy import function as nv | ||
|
@@ -921,7 +929,18 @@ def swaplevel(self: FrameOrSeries, i=-2, j=-1, axis=0) -> FrameOrSeries: | |
# ---------------------------------------------------------------------- | ||
# Rename | ||
|
||
def rename(self, *args, **kwargs): | ||
def rename( | ||
self: FrameOrSeries, | ||
mapper: Optional[Renamer] = None, | ||
*, | ||
index: Optional[Renamer] = None, | ||
columns: Optional[Renamer] = None, | ||
axis: Optional[Axis] = None, | ||
copy: bool = True, | ||
inplace: bool = False, | ||
level: Optional[Level] = None, | ||
errors: str = "ignore", | ||
) -> Optional[FrameOrSeries]: | ||
""" | ||
Alter axes input function or functions. Function / dict values must be | ||
unique (1-to-1). Labels not contained in a dict / Series will be left | ||
|
@@ -1034,44 +1053,46 @@ def rename(self, *args, **kwargs): | |
|
||
See the :ref:`user guide <basics.rename>` for more. | ||
""" | ||
axes, kwargs = self._construct_axes_from_arguments(args, kwargs) | ||
copy = kwargs.pop("copy", True) | ||
inplace = kwargs.pop("inplace", False) | ||
level = kwargs.pop("level", None) | ||
axis = kwargs.pop("axis", None) | ||
errors = kwargs.pop("errors", "ignore") | ||
if axis is not None: | ||
# Validate the axis | ||
self._get_axis_number(axis) | ||
|
||
if kwargs: | ||
raise TypeError( | ||
"rename() got an unexpected keyword " | ||
f'argument "{list(kwargs.keys())[0]}"' | ||
) | ||
|
||
if com.count_not_none(*axes.values()) == 0: | ||
if mapper is None and index is None and columns is None: | ||
raise TypeError("must pass an index to rename") | ||
|
||
self._consolidate_inplace() | ||
if index is not None or columns is not None: | ||
if axis is not None: | ||
raise TypeError( | ||
"Cannot specify both 'axis' and any of 'index' or 'columns'" | ||
) | ||
elif mapper is not None: | ||
raise TypeError( | ||
"Cannot specify both 'mapper' and any of 'index' or 'columns'" | ||
) | ||
else: | ||
# use the mapper argument | ||
if axis and self._get_axis_number(axis) == 1: | ||
columns = mapper | ||
else: | ||
index = mapper | ||
|
||
result = self if inplace else self.copy(deep=copy) | ||
|
||
# start in the axis order to eliminate too many copies | ||
for axis in range(self._AXIS_LEN): | ||
v = axes.get(self._AXIS_NAMES[axis]) | ||
if v is None: | ||
for axis_no, replacements in enumerate((index, columns)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ultimately I am trying to get rid of all of the internal If we buy into just having |
||
if replacements is None: | ||
continue | ||
f = com.get_rename_function(v) | ||
baxis = self._get_block_manager_axis(axis) | ||
|
||
ax = self._get_axis(axis_no) | ||
baxis = self._get_block_manager_axis(axis_no) | ||
f = com.get_rename_function(replacements) | ||
|
||
if level is not None: | ||
level = self.axes[axis]._get_level_number(level) | ||
level = ax._get_level_number(level) | ||
|
||
# GH 13473 | ||
if not callable(v): | ||
indexer = self.axes[axis].get_indexer_for(v) | ||
if not callable(replacements): | ||
indexer = ax.get_indexer_for(replacements) | ||
if errors == "raise" and len(indexer[indexer == -1]): | ||
missing_labels = [ | ||
label for index, label in enumerate(v) if indexer[index] == -1 | ||
label | ||
for index, label in enumerate(replacements) | ||
if indexer[index] == -1 | ||
] | ||
raise KeyError(f"{missing_labels} not found in axis") | ||
|
||
|
@@ -1082,6 +1103,7 @@ def rename(self, *args, **kwargs): | |
|
||
if inplace: | ||
self._update_inplace(result._data) | ||
return None | ||
else: | ||
return result.__finalize__(self) | ||
|
||
|
@@ -4036,7 +4058,7 @@ def add_prefix(self: FrameOrSeries, prefix: str) -> FrameOrSeries: | |
f = functools.partial("{prefix}{}".format, prefix=prefix) | ||
|
||
mapper = {self._info_axis_name: f} | ||
return self.rename(**mapper) | ||
return self.rename(**mapper) # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't seem to get dict unpacking to place nicely with mypy. @simonjayhawkins |
||
|
||
def add_suffix(self: FrameOrSeries, suffix: str) -> FrameOrSeries: | ||
""" | ||
|
@@ -4095,7 +4117,7 @@ def add_suffix(self: FrameOrSeries, suffix: str) -> FrameOrSeries: | |
f = functools.partial("{}{suffix}".format, suffix=suffix) | ||
|
||
mapper = {self._info_axis_name: f} | ||
return self.rename(**mapper) | ||
return self.rename(**mapper) # type: ignore | ||
|
||
def sort_values( | ||
self, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3124,7 +3124,7 @@ def argsort(self, axis=0, kind="quicksort", order=None): | |
|
||
Parameters | ||
---------- | ||
axis : int | ||
axis : {0 or "index"} | ||
Has no effect but is accepted for compatibility with numpy. | ||
kind : {'mergesort', 'quicksort', 'heapsort'}, default 'quicksort' | ||
Choice of sorting algorithm. See np.sort for more | ||
|
@@ -3893,7 +3893,16 @@ def align( | |
broadcast_axis=broadcast_axis, | ||
) | ||
|
||
def rename(self, index=None, **kwargs): | ||
def rename( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Series method doesn't completely align with the DataFrame method, but I think this is an easy way to go about it while maintain backwards compat There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the value in accepting I don't see an easy way to accept So maybe my question is: should we also accept There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we validate axis at all, or just ignore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here axis gets ignored. Looks like this was implemented as part of #18589
Yea I'm not sure myself. I was thinking ideally it would have been great to align the signatures better but for where we are that would cause some API churn. So I guess best to leave unless there's a ringing need to do something here (?) |
||
self, | ||
index=None, | ||
*, | ||
axis=None, | ||
copy=True, | ||
inplace=False, | ||
level=None, | ||
errors="ignore", | ||
): | ||
""" | ||
Alter Series index labels or name. | ||
|
||
|
@@ -3907,6 +3916,8 @@ def rename(self, index=None, **kwargs): | |
|
||
Parameters | ||
---------- | ||
axis : {0 or "index"} | ||
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. | ||
|
@@ -3924,6 +3935,7 @@ def rename(self, index=None, **kwargs): | |
|
||
See Also | ||
-------- | ||
DataFrame.rename : Corresponding DataFrame method. | ||
Series.rename_axis : Set the name of the axis. | ||
|
||
Examples | ||
|
@@ -3950,12 +3962,12 @@ def rename(self, index=None, **kwargs): | |
5 3 | ||
dtype: int64 | ||
""" | ||
kwargs["inplace"] = validate_bool_kwarg(kwargs.get("inplace", False), "inplace") | ||
|
||
if callable(index) or is_dict_like(index): | ||
return super().rename(index=index, **kwargs) | ||
return super().rename( | ||
index, copy=copy, inplace=inplace, level=level, errors=errors | ||
) | ||
else: | ||
return self._set_name(index, inplace=kwargs.get("inplace")) | ||
return self._set_name(index, inplace=inplace) | ||
|
||
@Substitution(**_shared_doc_kwargs) | ||
@Appender(generic.NDFrame.reindex.__doc__) | ||
|
Uh oh!
There was an error while loading. Please reload this page.