COMPAT-18589: Supporting axis in Series.rename #18923
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18923 +/- ##
==========================================
+ Coverage 91.58% 91.59% +<.01%
==========================================
Files 150 150
Lines 48806 48809 +3
==========================================
+ Hits 44701 44706 +5
+ Misses 4105 4103 -2
Continue to review full report at Codecov.
|
pandas/core/generic.py
Outdated
| @@ -862,6 +862,8 @@ def rename(self, *args, **kwargs): | |||
| copy = kwargs.pop('copy', True) | |||
| inplace = kwargs.pop('inplace', False) | |||
| level = kwargs.pop('level', None) | |||
| # Axis supported for compatibility, detailed in GH-18589 | |||
There was a problem hiding this comment.
you don't need this comment
pandas/tests/generic/test_series.py
Outdated
| def test_axis_supported(self): | ||
| # Supporting axis for compatibility, detailed in GH-18589 | ||
| s = Series(range(5)) | ||
| s.rename({}, axis=0) |
There was a problem hiding this comment.
test with axis > 0, which will raise
There was a problem hiding this comment.
move to pandas/tests/series/test_alter_axes.py with other rename tests.
There was a problem hiding this comment.
Moved tests, can you clarify on the comment about axis > 0 raising?
I added a test but it doesn't raise, we're popping it and never using it - did you mean that it should raise an error? I can implement if that's intended.
There was a problem hiding this comment.
there should a call of
_getaxis_number
somewhere if not that’s an error
There was a problem hiding this comment.
There's no call to that here, as we're not using the argument anywhere.
Would you still like me to add this (and test that the addition is working as intended) despite the argument not being used?
jreback
added
the
Compat
| @@ -862,6 +862,8 @@ def rename(self, *args, **kwargs): | |||
| copy = kwargs.pop('copy', True) | |||
| inplace = kwargs.pop('inplace', False) | |||
| level = kwargs.pop('level', None) | |||
There was a problem hiding this comment.
I think there is an issue about explicity listing the signatures in pandas/core/series and pandas/core/frame, for rename. would take that in this PR as well.
There was a problem hiding this comment.
Sure, happy to do so, just to clarify, you'd like to me to implement them as specific kwargs rather than **kwargs, and default them to None?
There was a problem hiding this comment.
defaults should be to what they actually default now, these are passed as kwargs to the generic routine, this is mainly to have a nice signature.
| # Supporting axis for compatibility, detailed in GH-18589 | ||
| s = Series(range(5)) | ||
| s.rename({}, axis=0) | ||
| s.rename({}, axis=5) |
pandas/core/generic.py
Outdated
| @@ -862,6 +862,9 @@ def rename(self, *args, **kwargs): | |||
| copy = kwargs.pop('copy', True) | |||
| inplace = kwargs.pop('inplace', False) | |||
| level = kwargs.pop('level', None) | |||
| axis = kwargs.pop('axis', None) | |||
| if axis is not None: | |||
| self._get_axis_number(axis) | |||
There was a problem hiding this comment.
axis = self._get_axis_number(axis)
| def test_rename_axis_supported(self): | ||
| # Supporting axis for compatibility, detailed in GH-18589 | ||
| s = Series(range(5)) | ||
| s.rename({}, axis=0) |
There was a problem hiding this comment.
try with axis='index' as welll (should succeed)
There was a problem hiding this comment.
can you move this after other rename tests?
| def test_rename_axis_supported(self): | ||
| # Supporting axis for compatibility, detailed in GH-18589 | ||
| s = Series(range(5)) | ||
| s.rename({}, axis=0) |
There was a problem hiding this comment.
can you move this after other rename tests?
|
thanks! |
git diff upstream/master -u -- "*.py" | flake8 --diff