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

Add inplace support for rename_axis #16505

Merged
merged 2 commits into from
Jun 13, 2017

Conversation

hendrikmakait
Copy link
Contributor

@codecov
Copy link

codecov bot commented May 26, 2017

Codecov Report

Merging #16505 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16505      +/-   ##
==========================================
+ Coverage   90.43%   90.43%   +<.01%     
==========================================
  Files         161      161              
  Lines       51045    51047       +2     
==========================================
+ Hits        46161    46163       +2     
  Misses       4884     4884
Flag Coverage Δ
#multiple 88.27% <100%> (ø) ⬆️
#single 40.16% <16.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/generic.py 92.17% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e81f3cc...7011067. Read the comment docs.

@codecov
Copy link

codecov bot commented May 26, 2017

Codecov Report

Merging #16505 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16505      +/-   ##
==========================================
- Coverage   90.95%   90.92%   -0.03%     
==========================================
  Files         161      161              
  Lines       49276    49278       +2     
==========================================
- Hits        44817    44807      -10     
- Misses       4459     4471      +12
Flag Coverage Δ
#multiple 88.68% <100%> (-0.03%) ⬇️
#single 40.18% <16.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/generic.py 92.27% <100%> (ø) ⬆️
pandas/plotting/_converter.py 63.23% <0%> (-1.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 344cec7...01eaba1. Read the comment docs.


Parameters
----------
name : str or list of str
Name for the Index, or list of names for the MultiIndex
axis : int or str
0 or 'index' for the index; 1 or 'columns' for the columns
inplace : bool
whether to modify `self` directly or return a copy
Copy link
Contributor

Choose a reason for hiding this comment

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

versionadded tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which version should I specify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specified 0.20.2

Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.21.0

@@ -831,9 +833,11 @@ def _set_axis_name(self, name, axis=0):
axis = self._get_axis_number(axis)
idx = self._get_axis(axis).set_names(name)

renamed = self.copy(deep=True)
inplace = validate_bool_kwarg(inplace, 'inplace')
renamed = self if inplace else self.copy(deep=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

deep=True is the default so can remove that

@hendrikmakait hendrikmakait force-pushed the 15704-rename-axis-inplace branch 2 times, most recently from df3520d to dc8e38c Compare June 1, 2017 08:56

Parameters
----------
name : str or list of str
Name for the Index, or list of names for the MultiIndex
axis : int or str
0 or 'index' for the index; 1 or 'columns' for the columns
inplace : bool
whether to modify `self` directly or return a copy
Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.21.0

@@ -418,6 +418,13 @@ def test_rename(self):
pd.Index(['bar', 'foo'], name='name'))
assert renamed.index.name == renamer.index.name

def test_rename_axis(self):
renamed = self.frame.rename_axis('foo')
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number as a comment

@@ -418,6 +418,13 @@ def test_rename(self):
pd.Index(['bar', 'foo'], name='name'))
assert renamed.index.name == renamer.index.name

def test_rename_axis(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

test_rename_axis_inplace

no_return = self.frame.rename_axis('foo', inplace=True)
assert no_return is None
renamed2 = self.frame
assert_frame_equal(renamed, renamed2)
Copy link
Contributor

Choose a reason for hiding this comment

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

make this read

assert_frame_equal(result, expected) (rename things)

@hendrikmakait hendrikmakait force-pushed the 15704-rename-axis-inplace branch from dc8e38c to 2c51767 Compare June 1, 2017 11:52
@@ -831,9 +835,11 @@ def _set_axis_name(self, name, axis=0):
axis = self._get_axis_number(axis)
idx = self._get_axis(axis).set_names(name)

renamed = self.copy(deep=True)
inplace = validate_bool_kwarg(inplace, 'inplace')
renamed = self if inplace else self.copy()
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you removed the deep=True ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the default and can thus be removed (as of @jreback's request that disappeared after my rebase).
https://github.com/pandas-dev/pandas/blob/master/pandas/core/generic.py#L3523

Copy link
Member

Choose a reason for hiding this comment

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

OK, perfect! (I looked for a comment about it, but didn't find one :-))

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

pls add a whatnews note in other enhancements for 0.21.0. ping on green.

@jreback jreback added the Indexing Related to indexing on series/frames, not to indexes themselves label Jun 1, 2017
@@ -418,6 +418,14 @@ def test_rename(self):
pd.Index(['bar', 'foo'], name='name'))
assert renamed.index.name == renamer.index.name

def test_rename_axis_inplace(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also add a similar test in series/test_alter_axes.py

and tests this with axis=1 as well.

@@ -33,6 +33,7 @@ Other Enhancements
- The ``validate`` argument for :func:`merge` function now checks whether a merge is one-to-one, one-to-many, many-to-one, or many-to-many. If a merge is found to not be an example of specified merge type, an exception of type ``MergeError`` will be raised. For more, see :ref:`here <merging.validation>` (:issue:`16270`)
- ``Series.to_dict()`` and ``DataFrame.to_dict()`` now support an ``into`` keyword which allows you to specify the ``collections.Mapping`` subclass that you would like returned. The default is ``dict``, which is backwards compatible. (:issue:`16122`)
- ``RangeIndex.append`` now returns a ``RangeIndex`` object when possible (:issue:`16212`)
- ``Series.rename_axis()`` and ``DataFrame.rename_axis()`` with ``inplace=True`` now return None while renaming the axis inplace. (:issue:`15704`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback - added

@hendrikmakait hendrikmakait force-pushed the 15704-rename-axis-inplace branch from 6ce4726 to 6da584b Compare June 10, 2017 22:20
@hendrikmakait hendrikmakait force-pushed the 15704-rename-axis-inplace branch from 6da584b to 01eaba1 Compare June 13, 2017 21:06
@jreback jreback merged commit 466e425 into pandas-dev:master Jun 13, 2017
@jreback
Copy link
Contributor

jreback commented Jun 13, 2017

thanks @hndrkmkt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rename_axis inplace=True
3 participants