ENH: level keyword in rename (GH4160) #13766
Conversation
jorisvandenbossche
force-pushed
the
rename-level
branch
from
74090c1 to
c63d51a
Compare
|
|
||
| """ | ||
| if isinstance(index, MultiIndex): | ||
| items = [tuple(func(y) for y in x) for x in index] | ||
| if level is not None: | ||
| items = [tuple(func(y) if i == level else y |
There was a problem hiding this comment.
this shoukd be in a rename method in Index
MI should also just iterate in the levels and call rename on that Index
should not be in internals
There was a problem hiding this comment.
Well, that's how rename is implemented at the moment ..
And AFAIK there is not a rename method on Index itself? (there is, but that is to change the name attribute, so something else)
There was a problem hiding this comment.
no I mean move it there
much cleaner to do it
There was a problem hiding this comment.
So I mean this comment. Do you mean it would be cleaner to have a method on Index/MultiIndex itself that does this renaming?
I can agree with that, but the problem is that there already is a 'rename' method which renames the names of a multi index (it could of course also be a private method)
But my preference is to leave the existing implementation intact in this PR (I just expanded the existing method a bit).
There was a problem hiding this comment.
yes I mean move this to MultiIndex, and create a corresponding (private is fine) one for Index. ideally it would be rename, but I guess has to be a private method.
|
this looks pretty good. maybe rebase and see where we are. |
|
Trying to revive this. @jreback I am not sure anymore what was missing before, but can you clarify your comment a bit? |
jorisvandenbossche
force-pushed
the
rename-level
branch
3 times, most recently
from
2df75a3 to
0065973
Compare
Codecov Report
@@ Coverage Diff @@
## master #13766 +/- ##
==========================================
+ Coverage 91% 91% +<.01%
==========================================
Files 145 145
Lines 49581 49586 +5
==========================================
+ Hits 45122 45128 +6
+ Misses 4459 4458 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #13766 +/- ##
==========================================
+ Coverage 90.99% 90.99% +<.01%
==========================================
Files 153 153
Lines 50481 50486 +5
==========================================
+ Hits 45937 45942 +5
Misses 4544 4544
Continue to review full report at Codecov.
|
jorisvandenbossche
force-pushed
the
rename-level
branch
from
0065973 to
4c660af
Compare
jorisvandenbossche
force-pushed
the
rename-level
branch
from
4c660af to
1915696
Compare
|
@jreback I merged this, as I currently don't have time to address the clean-up comment (and I just used what was already in place). Do you find it worth to open an issue to keep track of that comment? |
|
create a new issue to track this and mark it for 0.20.0 I will get to this. This impl is not consistent with how things are done internally. |
git diff upstream/master | flake8 --diff