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

ENH/API: Scalar values for Series.rename and NDFrame.rename_axis #11980

Closed

Conversation

TomAugspurger
Copy link
Contributor

closes #9494
closes #11965

Okay refactored to overload NDFrame.rename and NDFrame.rename_axis

New rules:

  • Series.rename(scalar) or Series.rename(list-like) will alter Series.name
  • NDFrame.rename(mapping) and NDFrame.rename(function) operate as before (alter labels)
  • NDFrame.rename_axis(scaler) or NDFrame.rename_axis(list-like) will change the Index.name or MutliIndex.names
  • NDFrame.rename_axis(mapping) and NDFrame.rename_axis(function) operate as before.

I don't like overloading the method like this, but rename and rename_axis are the right method names. It's mostly backwards compatible, aside from people doing things like try: series.rename(x): except TypeError: ... and I don't know what our guarantees are around things like that.

@TomAugspurger TomAugspurger added this to the 0.18.0 milestone Jan 7, 2016
@@ -367,6 +367,7 @@ Reindexing / Selection / Label manipulation
Series.reindex
Series.reindex_like
Series.rename
Series.set_axis_name
Series.reset_index
Copy link
Contributor

Choose a reason for hiding this comment

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

add .set_name

@jreback
Copy link
Contributor

jreback commented Jan 7, 2016

I would add an example (like this)

maybe in the data structures section.

@jreback jreback added Indexing Related to indexing on series/frames, not to indexes themselves MultiIndex labels Jan 7, 2016
@shoyer
Copy link
Member

shoyer commented Jan 7, 2016

Do we need an inplace argument for these new methods at all? You can already just mutate the index directly if you want to do that.

@TomAugspurger
Copy link
Contributor Author

OK, removing inplace.

Thoughts on pd.Series.rename(scalar) changing the Series name, and NDFrame.rename_axis(scalar/list-like) changing the axis name(s)? Those methods will keep their behavior for mappings / function inputs IIUC this is what xray does?

@jreback
Copy link
Contributor

jreback commented Jan 7, 2016

that would be quite an API change

@TomAugspurger
Copy link
Contributor Author

I think it'd be backwards compatible right? Right now we raise a TypeError on scalars / array-likes. I'll writeup an example later.

"""
is_scalar_or_list = (
(not com.is_sequence(mapper) and not callable(mapper)) or
(com.is_list_like(mapper) and not isinstance(mapper,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have a check for is_dict_like in core/common? I couldn't find one, and can move this or something similar into there if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a is_dict_like and just make it a check for https://docs.python.org/2/library/collections.html#collections.MutableMapping

Copy link
Member

Choose a reason for hiding this comment

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

Better to use duck typing -- for example, I'm pretty sure Series would fail that test :).

I wrote one of these for xarray: https://github.com/pydata/xarray/blob/v0.7.0/xarray/core/utils.py#L137

Copy link
Contributor

Choose a reason for hiding this comment

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

I find is_list_like & co ambiguous, because I don't know if list means to me what it means to the person who wrote it. the function name should describe the properties being tested for (e.g. is_immuatable_sequence rather than is_tuple_like). #12056 is properly the place to discuss that.

@TomAugspurger
Copy link
Contributor Author

FYI, changed the API around, see the updated original post, which makes it more like xray

Basically we look at the type of the argument. For dict/functions we alter labels as before. For scalar/list-like we alter Series/index name(s).

@jorisvandenbossche
Copy link
Member

Question: what would be the ideal name for the original functionality (i.e. changing the column names/index labels) if we could choose it know? Would that be something else as rename?

If we think of something really better, we could also opt to add that as a method, and at least use two different functions in the documentation for these two different operation that we can recommend to use (kind of soft deprecation)

@jorisvandenbossche
Copy link
Member

Further, one drawback of the overloading of rename, is that you cannot rename a certain level of a MultiIndex using a mapping like {'original_level_name': 'new_level_name'} (instead of having to specify the level you want to rename with level=..)

@TomAugspurger
Copy link
Contributor Author

what would be the ideal name for the original functionality

relabel I think.

And you're correct not being able to rename a single level of a MultiIndex; you'd have to specify all the levels, even the ones you aren't changing.. We could potentially add a level argument rename, but I think that's more confusion than it's worth and I didn't want to change the signature at all.

@jreback
Copy link
Contributor

jreback commented Jan 9, 2016

note #4160 for long time bug in renaming MultiIndex

Determin whether ``arg`` is dict-like.
'''
return hasattr(arg, '__getitem__') and hasattr(arg, 'keys')

Copy link
Contributor

Choose a reason for hiding this comment

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

does this work for py3 dictviews? (maybe add a confirming tests in test_common.py)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite sure what you mean. IIUC the only dictviews are (python3) dict.keys(), .items() and .values(), none of which are dict like.

@TomAugspurger TomAugspurger force-pushed the chainable-rename branch 2 times, most recently from 262aeb3 to 45b1484 Compare February 10, 2016 16:25
Overloading the APIs of Series.rename and
NDFramed.rename_axis for method chaining.
@TomAugspurger
Copy link
Contributor Author

@jreback could you expand a bit on what you mean by dictviews? I don't think that dict.keys() / dict.values() / dict.itms()

And on the docstrings, I don't think there's a way to only show certain examples for Series and others for DataFrames, is there? We have other docstrings showing both. I did have add an example with DataFrame.rename(scalar) raising a TypeError.

Other than those I think this is ready. The 2.7 build failed while building the docs because the xarray docstring you fixed in 4404d39

@jreback
Copy link
Contributor

jreback commented Feb 11, 2016

@TomAugspurger nvm, was thinking of handling the actual keys which are dict_keys IOW, views. but that is not what you are doing.

@TomAugspurger
Copy link
Contributor Author

Ok, thanks!

@jreback
Copy link
Contributor

jreback commented Feb 11, 2016

side issue: we might want to revisit the signature of .rename & .reindex to be more like

.rename(values, axis) more in-line with other methods I think.

e.g.

.rename(lambda x: str(x), axis=1) rather than .rename(columns=lambda x: str(x))
.reindex(values, axis=1) rather than .reindex(columns=values)

@TomAugspurger
Copy link
Contributor Author

Think that's worth breaking API for?

With rename we at least have rename_axis. Could point users to that if you want.

At the very least, it's nice that these two methods are at least similar to eachother. They're both for altering the axis labels, and they both take index=, columns-

@jreback
Copy link
Contributor

jreback commented Feb 11, 2016

actually we also have .reindex_axis so I guess .rename/.reindex are the axis-less ones....

cldy pushed a commit to cldy/pandas that referenced this pull request Feb 11, 2016
closes pandas-dev#9494  closes
pandas-dev#11965    Okay refactored to
overload `NDFrame.rename` and `NDFrame.rename_axis`    New rules:    -
Series.rename(scalar) or Series.rename(list-like) will alter
`Series.name`  - NDFrame.rename(mapping) and NDFrame.rename(function)
operate as before (alter labels)  - NDFrame.rename_axis(scaler) or
NDFrame.rename_axis(list-like) will change the Index.name or
MutliIndex.names  - NDFrame.rename_axis(mapping) and
NDFrame.rename_axis(function) operate as before.    I don't like
overloading the method like this, but `rename` and `rename_axis` are
the right method names. It's *mostly* backwards compatible, aside from
people doing things like `try: series.rename(x): except TypeError:
...` and I don't know what our guarantees are around things like that.

Author: Tom Augspurger <tom.w.augspurger@gmail.com>

Closes pandas-dev#11980 from TomAugspurger/chainable-rename and squashes the following commits:

b36df76 [Tom Augspurger] ENH/API: Series.rename and NDFrame.rename_axis
TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Mar 15, 2016
Closes pandas-dev#12623

I added com.is_dict_like in pandas-dev#11980
and failed to use it for the `rename` method. Using that now and did
some refactoring while I was in there. Added more tests for rename.
jreback pushed a commit that referenced this pull request Mar 17, 2016
closes #12623
closes #12626

I added com.is_dict_like in #11980
and failed to use it for the `rename` method. Using that now and did
some refactoring while I was in there. Added more tests for rename.
@TomAugspurger TomAugspurger deleted the chainable-rename branch November 3, 2016 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Indexing Related to indexing on series/frames, not to indexes themselves MultiIndex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add method to change the .name attribute on the axes Chainable Series.set_name()
5 participants