Skip to content

REF: Use * syntax to make reindex kwargs keyword only #50853

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

Merged
merged 14 commits into from
Jan 30, 2023
Merged
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
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v2.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ Removal of prior version deprecations/changes
- Disallow passing non-keyword arguments to :meth:`DataFrame.replace`, :meth:`Series.replace` except for ``to_replace`` and ``value`` (:issue:`47587`)
- Disallow passing non-keyword arguments to :meth:`DataFrame.sort_values` except for ``by`` (:issue:`41505`)
- Disallow passing non-keyword arguments to :meth:`Series.sort_values` (:issue:`41505`)
- Disallow passing 2 non-keyword arguments to :meth:`DataFrame.reindex` (:issue:`17966`)
- Disallow passing non-keyword arguments to :meth:`DataFrame.reindex` except for ``labels`` (:issue:`17966`)
- Disallow :meth:`Index.reindex` with non-unique :class:`Index` objects (:issue:`42568`)
- Disallowed constructing :class:`Categorical` with scalar ``data`` (:issue:`38433`)
- Disallowed constructing :class:`CategoricalIndex` without passing ``data`` (:issue:`38944`)
Expand Down
2 changes: 0 additions & 2 deletions pandas/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@ def pytest_collection_modifyitems(items, config) -> None:
ignored_doctest_warnings = [
# Docstring divides by zero to show behavior difference
("missing.mask_zero_div_zero", "divide by zero encountered"),
# Docstring demonstrates the call raises a warning
("_validators.validate_axis_style_args", "Use named arguments"),
]

for item in items:
Expand Down
68 changes: 42 additions & 26 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,10 @@
Appender,
Substitution,
doc,
rewrite_axis_style_signature,
)
from pandas.util._exceptions import find_stack_level
from pandas.util._validators import (
validate_ascending,
validate_axis_style_args,
validate_bool_kwarg,
validate_percentile,
)
Expand Down Expand Up @@ -260,11 +258,18 @@
levels and/or column labels.
- if `axis` is 1 or `'columns'` then `by` may contain column
levels and/or index labels.""",
"optional_labels": """labels : array-like, optional
New labels / index to conform the axis specified by 'axis' to.""",
"optional_axis": """axis : int or str, optional
Axis to target. Can be either the axis name ('index', 'columns')
or number (0, 1).""",
"optional_reindex": """
labels : array-like, optional
New labels / index to conform the axis specified by 'axis' to.
index : array-like, optional
New labels for the index. Preferably an Index object to avoid
duplicating data.
columns : array-like, optional
New labels for the columns. Preferably an Index object to avoid
duplicating data.
axis : int or str, optional
Axis to target. Can be either the axis name ('index', 'columns')
or number (0, 1).""",
"replace_iloc": """
This differs from updating with ``.loc`` or ``.iloc``, which require
you to specify a location to update with some value.""",
Expand Down Expand Up @@ -4990,26 +4995,37 @@ def set_axis(
) -> DataFrame:
return super().set_axis(labels, axis=axis, copy=copy)

@Substitution(**_shared_doc_kwargs)
@Appender(NDFrame.reindex.__doc__)
@rewrite_axis_style_signature(
"labels",
[
("method", None),
("copy", None),
("level", None),
("fill_value", np.nan),
("limit", None),
("tolerance", None),
],
@doc(
NDFrame.reindex,
klass=_shared_doc_kwargs["klass"],
optional_reindex=_shared_doc_kwargs["optional_reindex"],
)
def reindex(self, *args, **kwargs) -> DataFrame:
axes = validate_axis_style_args(self, args, kwargs, "labels", "reindex")
kwargs.update(axes)
# Pop these, since the values are in `kwargs` under different names
kwargs.pop("axis", None)
kwargs.pop("labels", None)
return super().reindex(**kwargs)
def reindex( # type: ignore[override]
self,
labels=None,
*,
index=None,
columns=None,
axis: Axis | None = None,
method: str | None = None,
copy: bool | None = None,
level: Level | None = None,
fill_value: Scalar | None = np.nan,
limit: int | None = None,
tolerance=None,
) -> DataFrame:
return super().reindex(
labels=labels,
index=index,
columns=columns,
axis=axis,
method=method,
copy=copy,
level=level,
fill_value=fill_value,
limit=limit,
tolerance=tolerance,
)

@overload
def drop(
Expand Down
72 changes: 41 additions & 31 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
NDFrameT,
RandomState,
Renamer,
Scalar,
SortKind,
StorageOptions,
Suffixes,
Expand Down Expand Up @@ -5104,11 +5105,21 @@ def sort_index(

@doc(
klass=_shared_doc_kwargs["klass"],
axes=_shared_doc_kwargs["axes"],
optional_labels="",
optional_axis="",
optional_reindex="",
)
def reindex(self: NDFrameT, *args, **kwargs) -> NDFrameT:
def reindex(
self: NDFrameT,
labels=None,
index=None,
columns=None,
axis: Axis | None = None,
method: str | None = None,
copy: bool_t | None = None,
level: Level | None = None,
fill_value: Scalar | None = np.nan,
limit: int | None = None,
tolerance=None,
) -> NDFrameT:
"""
Conform {klass} to new index with optional filling logic.

Expand All @@ -5118,11 +5129,7 @@ def reindex(self: NDFrameT, *args, **kwargs) -> NDFrameT:

Parameters
----------
{optional_labels}
{axes} : array-like, optional
New labels / index to conform to, should be specified using
keywords. Preferably an Index object to avoid duplicating data.
{optional_axis}
{optional_reindex}
method : {{None, 'backfill'/'bfill', 'pad'/'ffill', 'nearest'}}
Method to use for filling holes in reindexed DataFrame.
Please note: this is only applicable to DataFrames/Series with a
Expand Down Expand Up @@ -5311,31 +5318,34 @@ def reindex(self: NDFrameT, *args, **kwargs) -> NDFrameT:
# TODO: Decide if we care about having different examples for different
# kinds

# construct the args
axes, kwargs = self._construct_axes_from_arguments(args, kwargs)
method = clean_reindex_fill_method(kwargs.pop("method", None))
level = kwargs.pop("level", None)
copy = kwargs.pop("copy", None)
limit = kwargs.pop("limit", None)
tolerance = kwargs.pop("tolerance", None)
fill_value = kwargs.pop("fill_value", None)

# Series.reindex doesn't use / need the axis kwarg
# We pop and ignore it here, to make writing Series/Frame generic code
# easier
kwargs.pop("axis", None)

if kwargs:
raise TypeError(
"reindex() got an unexpected keyword "
f'argument "{list(kwargs.keys())[0]}"'
)
if index is not None and columns is not None and labels is not None:
raise TypeError("Cannot specify all of 'labels', 'index', 'columns'.")
elif 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'"
)
if labels is not None:
if index is not None:
columns = labels
else:
index = labels
else:
if axis and self._get_axis_number(axis) == 1:
columns = labels
else:
index = labels
axes: dict[Literal["index", "columns"], Any] = {
"index": index,
"columns": columns,
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 ATM "columns" is not included in Series cases. this doesn't break anything?

Copy link
Member Author

@mroeschke mroeschke Jan 25, 2023

Choose a reason for hiding this comment

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

Yeah columns should not be included in the Series case. Before the passed labels would be forced into the index in the Series case

}
method = clean_reindex_fill_method(method)

# if all axes that are requested to reindex are equal, then only copy
# if indicated must have index names equal here as well as values
if all(
self._get_axis(axis).identical(ax)
for axis, ax in axes.items()
self._get_axis(axis_name).identical(ax)
for axis_name, ax in axes.items()
if ax is not None
):
return self.copy(deep=copy)
Expand Down Expand Up @@ -5517,7 +5527,7 @@ def filter(
name = self._get_axis_name(axis)
# error: Keywords must be strings
return self.reindex( # type: ignore[misc]
**{name: [r for r in items if r in labels]}
**{name: [r for r in items if r in labels]} # type: ignore[arg-type]
)
elif like:

Expand Down
2 changes: 1 addition & 1 deletion pandas/core/groupby/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -4098,7 +4098,7 @@ def _reindex_output(
"copy": False,
"fill_value": fill_value,
}
return output.reindex(**d)
return output.reindex(**d) # type: ignore[arg-type]

# GH 13204
# Here, the categorical in-axis groupers, which need to be fully
Expand Down
45 changes: 29 additions & 16 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
NaPosition,
QuantileInterpolation,
Renamer,
Scalar,
SingleManager,
SortKind,
StorageOptions,
Expand Down Expand Up @@ -190,8 +191,12 @@
"duplicated": "Series",
"optional_by": "",
"optional_mapper": "",
"optional_labels": "",
"optional_axis": "",
"optional_reindex": """
index : array-like, optional
New labels for the index. Preferably an Index object to avoid
duplicating data.
axis : int or str, optional
Unused.""",
"replace_iloc": """
This differs from updating with ``.loc`` or ``.iloc``, which require
you to specify a location to update with some value.""",
Expand Down Expand Up @@ -4862,21 +4867,29 @@ def set_axis(
@doc(
NDFrame.reindex, # type: ignore[has-type]
klass=_shared_doc_kwargs["klass"],
axes=_shared_doc_kwargs["axes"],
optional_labels=_shared_doc_kwargs["optional_labels"],
optional_axis=_shared_doc_kwargs["optional_axis"],
optional_reindex=_shared_doc_kwargs["optional_reindex"],
)
def reindex(self, *args, **kwargs) -> Series:
if len(args) > 1:
raise TypeError("Only one positional argument ('index') is allowed")
if args:
(index,) = args
if "index" in kwargs:
raise TypeError(
"'index' passed as both positional and keyword argument"
)
kwargs.update({"index": index})
return super().reindex(**kwargs)
def reindex( # type: ignore[override]
self,
index=None,
*,
axis: Axis | None = None,
method: str | None = None,
copy: bool | None = None,
level: Level | None = None,
fill_value: Scalar | None = None,
limit: int | None = None,
tolerance=None,
) -> Series:
return super().reindex(
index=index,
method=method,
copy=copy,
level=level,
fill_value=fill_value,
limit=limit,
tolerance=tolerance,
)

@overload
def drop(
Expand Down
9 changes: 5 additions & 4 deletions pandas/tests/frame/methods/test_reindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -841,17 +841,18 @@ def test_reindex_positional_raises(self):
# https://github.com/pandas-dev/pandas/issues/12392
# Enforced in 2.0
df = DataFrame({"A": [1, 2, 3], "B": [4, 5, 6]})
with pytest.raises(TypeError, match=r".* is ambiguous."):
msg = r"reindex\(\) takes from 1 to 2 positional arguments but 3 were given"
with pytest.raises(TypeError, match=msg):
df.reindex([0, 1], ["A", "B", "C"])

def test_reindex_axis_style_raises(self):
# https://github.com/pandas-dev/pandas/issues/12392
df = DataFrame({"A": [1, 2, 3], "B": [4, 5, 6]})
with pytest.raises(TypeError, match="Cannot specify both 'axis'"):
df.reindex([0, 1], ["A"], axis=1)
df.reindex([0, 1], columns=["A"], axis=1)

with pytest.raises(TypeError, match="Cannot specify both 'axis'"):
df.reindex([0, 1], ["A"], axis="index")
df.reindex([0, 1], columns=["A"], axis="index")

with pytest.raises(TypeError, match="Cannot specify both 'axis'"):
df.reindex(index=[0, 1], axis="index")
Expand All @@ -866,7 +867,7 @@ def test_reindex_axis_style_raises(self):
df.reindex(index=[0, 1], columns=[0, 1], axis="columns")

with pytest.raises(TypeError, match="Cannot specify all"):
df.reindex([0, 1], [0], ["A"])
df.reindex(labels=[0, 1], index=[0], columns=["A"])

# Mixing styles
with pytest.raises(TypeError, match="Cannot specify both 'axis'"):
Expand Down
7 changes: 3 additions & 4 deletions pandas/tests/series/methods/test_reindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,16 +369,15 @@ def test_reindex_periodindex_with_object(p_values, o_values, values, expected_va
def test_reindex_too_many_args():
# GH 40980
ser = Series([1, 2])
with pytest.raises(
TypeError, match=r"Only one positional argument \('index'\) is allowed"
):
msg = r"reindex\(\) takes from 1 to 2 positional arguments but 3 were given"
with pytest.raises(TypeError, match=msg):
ser.reindex([2, 3], False)


def test_reindex_double_index():
# GH 40980
ser = Series([1, 2])
msg = r"'index' passed as both positional and keyword argument"
msg = r"reindex\(\) got multiple values for argument 'index'"
with pytest.raises(TypeError, match=msg):
ser.reindex([2, 3], index=[3, 4])

Expand Down
Loading