API: Added axis argument to rename, reindex
State: closed

xref: https://github.com/pandas-dev/pandas/issues/12392

I want to test this a bit further and clean up the code for a bit, but figured I'd put this up as I'm going offline for a few hours.

Any comments on the general approach of adding `*args`, and then manually validating that the parameters are consistent?

Labels:
name: API Design

Comments:
Ahh, I forgot that was keyword-only args are python3 only :/
OK, this is closer to @jorisvandenbossche's suggesting of adding a new parameter at the beginning. I chose `mapper` for this, it'l be `labels` for `reindex`.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=h1) Report
> Merging [#17800](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/22515f58c178cdb5cd38c4e56f26dc91c7053550?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `82.6%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17800/graphs/tree.svg?token=eZ4WkYLtcO&height=150&width=650&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17800      +/-   ##
==========================================
- Coverage   91.24%   91.22%   -0.03%     
==========================================
  Files         163      163              
  Lines       49967    49993      +26     
==========================================
+ Hits        45593    45606      +13     
- Misses       4374     4387      +13
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.03% <82.6%> (-0.01%)` | :arrow_down: |
| #single | `40.24% <30.43%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.89% <ø> (ø)` | :arrow_up: |
| [pandas/core/panel.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=) | `96.94% <ø> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.76% <100%> (-0.09%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `91.85% <33.33%> (-0.19%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/internals.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzLnB5) | `94.38% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=footer). Last update [22515f5...7ac2e3b](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=h1) Report
> Merging [#17800](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/d12a7a01888258a4c95fa7003916859f110075f1?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17800/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17800      +/-   ##
==========================================
+ Coverage   91.22%   91.22%   +<.01%     
==========================================
  Files         163      163              
  Lines       49994    50009      +15     
==========================================
+ Hits        45606    45621      +15     
  Misses       4388     4388
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.03% <100%> (+0.02%)` | :arrow_up: |
| #single | `40.25% <46.42%> (-0.06%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/panel.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=) | `96.94% <ø> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.88% <ø> (-0.1%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `92.03% <ø> (-0.06%)` | :arrow_down: |
| [pandas/core/sparse/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3Nlcmllcy5weQ==) | `95.26% <ø> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.77% <100%> (-0.08%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.58% <0%> (+0.09%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=footer). Last update [d12a7a0...0e668bf](https://codecov.io/gh/pandas-dev/pandas/pull/17800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

DataFrame docstring:

```
Signature: df.rename(mapper=None, index=None, columns=None, axis=None, **kwargs)
Docstring:
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
as-is. Extra labels listed don't throw an error. Alternatively, change
``Series.name`` with a scalar value (Series only).

Parameters
----------
mapper : dict-like or function
    Applied to the axis specified by `axis`
index, columns : scalar, list-like, dict-like or function, optional
    Scalar or list-like will alter the ``Series.name`` attribute,
    and raise on DataFrame or Panel.
    dict-like or functions are transformations to apply to
    that axis' values
copy : boolean, default True
    Also copy underlying data
inplace : boolean, default False
    Whether to return a new DataFrame. If True then value of copy is
    ignored.
level : int or level name, default None
    In case of a MultiIndex, only rename labels in the specified
    level.

Returns
-------
renamed : DataFrame (new object)
...
```


Examples:

```
In [1]: import pandas as pd

In [2]: df = pd.DataFrame({"A": [1, 2], "B": ['a', 'b'], 'C': [1, 2]})

In [4]: df.rename(id)
Out[4]:
            A  B  C
4370471600  1  a  1
4370471632  2  b  2

In [5]: df.rename(id, axis=1)
Out[5]:
   4373200824 4372848512  4372782920
0           1          a           1
1           2          b           2

In [6]: df.rename(id, id)
/Users/taugspurger/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/core/frame.py:2936: UserWarning: Interpreting call to '.rename(a, b)' as '.rename(index=a, columns=b)'. Use keyword arguments to remove ambiguity.
  UserWarning)
Out[6]:
            4373200824 4372848512  4372782920
4370471600           1          a           1
4370471632           2          b           2

In [7]: df.rename(index=id, columns=id)
Out[7]:
            4373200824 4372848512  4372782920
4370471600           1          a           1
4370471632           2          b           2

In [8]: df.rename(index=id, axis=1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-8-8a1d2adfffdf> in <module>()
----> 1 df.rename(index=id, axis=1)

~/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/core/frame.py in rename(self, mapper, index, columns, axis, **kwargs)
   2916             axis = self._get_axis_name(axis)
   2917             if index is not None or columns is not None:
-> 2918                 raise TypeError("Can't specify both 'axis' and 'index' or "
   2919                                 "'columns' Specify either\n"
   2920                                 "\t.rename(mapper, axis=axis), or\n"

TypeError: Can't specify both 'axis' and 'index' or 'columns' Specify either
        .rename(mapper, axis=axis), or
        .rename(index=index, columns=columns)

In [9]: df.rename(columns=id, axis=1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-2dd7c9eb0abf> in <module>()
----> 1 df.rename(columns=id, axis=1)

~/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/core/frame.py in rename(self, mapper, index, columns, axis, **kwargs)
   2916             axis = self._get_axis_name(axis)
   2917             if index is not None or columns is not None:
-> 2918                 raise TypeError("Can't specify both 'axis' and 'index' or "
   2919                                 "'columns' Specify either\n"
   2920                                 "\t.rename(mapper, axis=axis), or\n"

TypeError: Can't specify both 'axis' and 'index' or 'columns' Specify either
        .rename(mapper, axis=axis), or
        .rename(index=index, columns=columns)

In [10]: df.rename(index=id, columns=id, axis=1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-10-a15c6c06cf1d> in <module>()
----> 1 df.rename(index=id, columns=id, axis=1)

~/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/core/frame.py in rename(self, mapper, index, columns, axis, **kwargs)
   2916             axis = self._get_axis_name(axis)
   2917             if index is not None or columns is not None:
-> 2918                 raise TypeError("Can't specify both 'axis' and 'index' or "
   2919                                 "'columns' Specify either\n"
   2920                                 "\t.rename(mapper, axis=axis), or\n"

TypeError: Can't specify both 'axis' and 'index' or 'columns' Specify either
        .rename(mapper, axis=axis), or
        .rename(index=index, columns=columns)

In [11]: df.rename(mapper=id, index=id, columns=id)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-11-0175716afaa0> in <module>()
----> 1 df.rename(mapper=id, index=id, columns=id)

~/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/core/frame.py in rename(self, mapper, index, columns, axis, **kwargs)
   2925                 columns = mapper
   2926         elif all(x is not None for x in (mapper, index, columns)):
-> 2927             raise TypeError("Cannot specify all of 'mapper', 'index', and "
   2928                             "'columns'. Specify 'mapper' and 'axis', or "
   2929                             "'index' and 'columns'.")

TypeError: Cannot specify all of 'mapper', 'index', and 'columns'. Specify 'mapper' and 'axis', or 'index' and 'columns'.
```

The most concerning part (to me) is the one emitting a warning. I interpret `df.rename(id, id)`, as `df.rename(index=id, columns=id)`, which was the previous signature. This means we *also* interpret `df.rename(mapper=id, index=id)` as `df.rename(index=id, columns=id)`, which feels *wrong*, but I think is unavoidable.
Probably too late, but in case it can still be used as inspiration, my original branch: https://github.com/pandas-dev/pandas/compare/master...jorisvandenbossche:harmonize-reindex-drop-original?expand=1 (from a long time ago, in the recent PR I removed the `reindex` related changes and only cleaned-up `drop`)
Are you also going to do `reindex` ? In that case, you can copy the tests from the branch I linked to above (and maybe also code)
> Are you also going to do reindex ? 

Working on it now. https://github.com/pandas-dev/pandas/pull/17800/commits/1e62a0e9890d7c23d19b7eaa9b91d24b466ac36a refactored the common bits out. Perhaps I'll just push my reindex branch in this PR? They're similar enough.

I'll take your tests.
> Perhaps I'll just push my reindex branch in this PR? They're similar enough.

As you prefer, no problem to have them both here (certainly if they would share code to do the validation)
Ok (force) pushed two clean commits, one for rename and one for reindex.

I think this should be ready for review.
Trying to figure out the failing Panel test https://travis-ci.org/pandas-dev/pandas/jobs/284200489#L1392 now (cc @jreback)

Edit: nvm, I think I found it.
Got all the doc comments addressed I think.

I haven't had time to clean up the implementation to either refactor the changes to `.drop` to share code, or to use Joris' suggestion of using axis-style instead of index/columns.
@TomAugspurger ping when ready to have a look
I think it's ready.

I still haven't had time to refactor the implementation, and probably won't have time before the RC.
Fixups:

1. Dedicated docstrings for Series.rename and DataFrame.rename. I bit of duplication, but agreed w/ @jorisvandenbossche that it's better
2. Changed the default `axis` from `0` to `None` for DataFrame.reindex and rename. I think it's better to catch this.
lgtm merge when ready (rebase)
@TomAugspurger Thanks a lot for finishing this! 

Any opinion on https://github.com/pandas-dev/pandas/issues/17833 ?
One backwards-incompatible change that I didn't consider, using

```python
In [8]: df = pd.DataFrame(columns=['A', 'B'])

In [10]: df.rename(None, str.lower)
Out[10]:
Empty DataFrame
Columns: [A, B]
Index: []
```

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 :/