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

Conversation

mroeschke
Copy link
Member

After enforcing #17966, this makes DataFrame.reindex effectively keyword only except for labels, so removed ad-hoc logic to enforce that. Series.reindex also has similar ad-hoc logic which can be removed.

Also did some docstring cleanup for both functions.

@mroeschke mroeschke added the Refactor Internal refactoring of code label Jan 19, 2023
@mroeschke mroeschke changed the title REF: Use * syntax to make reindex kwags keyword only REF: Use * syntax to make reindex kwargs keyword only Jan 19, 2023
@mroeschke mroeschke added this to the 2.0 milestone Jan 24, 2023
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

@jbrockmendel
Copy link
Member

docstrings left unchanged?

@mroeschke
Copy link
Member Author

mroeschke commented Jan 25, 2023

docstrings left unchanged?

Yea but slightly improved. Went from

keywords for axesarray-like, optional

    New labels / index to conform to, should be specified using keywords. Preferably an Index object to avoid duplicating data.

to more specifically what is used e.g. DataFrame.reindex

labels : array-like, optional
    New labels / index to conform the axis specified by 'axis'.
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).

@mroeschke
Copy link
Member Author

Going to merge, but can follow up if needed

@mroeschke mroeschke merged commit cc6c957 into pandas-dev:main Jan 30, 2023
@mroeschke mroeschke deleted the ref/reindex/positional branch January 30, 2023 20:07
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Jan 30, 2023
* Use newer positional only syntax

* Push logic into generic, improve docs

* Clarify whatsnew

* Fix shared_doc

* Make axis more generic

* Series has different default

* Add some type ignores

* Fix docstring validation

* doc validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants