Skip to content
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

API break with DataFrame.rename #17963

Closed
TomAugspurger opened this issue Oct 24, 2017 · 1 comment · Fixed by #17966
Closed

API break with DataFrame.rename #17963

TomAugspurger opened this issue Oct 24, 2017 · 1 comment · Fixed by #17966
Milestone

Comments

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Oct 24, 2017

Posted in #17800 (comment), but making a new issue to make sure it's not lost and that everone is aware, and to check if we're comfortable with this:

df = pd.DataFrame(columns=['A', 'B'])
df.rename(None, str.lower)

In previous versions that meant .rename(index=None, columns=str.lower), but now it's .rename(mapper=None, index=str.lower). I think this is fundamentally ambiguous correct? This is breaking dask, which used positional arguments, but I suspect it'll break more code.

cc @jreback @jorisvandenbossche

@TomAugspurger TomAugspurger added this to the 0.21.0 milestone Oct 24, 2017
@TomAugspurger
Copy link
Contributor Author

Thinking out loud here:

We could change the signature of DataFrame.rename to (*args, **kwargs). Then

  1. Call with all keyword arguments, no ambiguity (ideal case)
  2. Call with positional arguments. We interpret that as .rename(index=args[0], columns=args[1]) and warn telling them to use keyword args
  3. Call with 1 positional argument, and an axis= kwarg. No problem
  4. Everything else raises?

downside: we'll need to rewrite DataFrame.__signature__ so that introspection works properly, won't be too bad. Let me see if that works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant