-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: GH #12223, GH #15262. Allow ints for names in MultiIndex #15478
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
Conversation
did u mean to close? |
@jreback yes. The conflicts indicated you had made a change that was incompatible with what I did, so I need to fix that. Will create a new pull request when ready. |
@Dr-Irv you don't need to close a PR to fix that though. simply rebase and force push. you never need to open a new PR for the same exact issue. |
@jreback OK, didn't know that. Should I reopen this one when I'm ready? |
sure |
@Dr-Irv Already reopened it, because if you push again while the PR is closed, you cannot reopen it anymore. |
doc/source/whatsnew/v0.20.0.txt
Outdated
@@ -185,11 +185,12 @@ Other enhancements | |||
- ``Series/DataFrame.asfreq()`` have gained a ``fill_value`` parameter, to fill missing values (:issue:`3715`). | |||
- ``Series/DataFrame.resample.asfreq`` have gained a ``fill_value`` parameter, to fill missing values during resampling (:issue:`3715`). | |||
- ``pandas.tools.hashing`` has gained a ``hash_tuples`` routine, and ``hash_pandas_object`` has gained the ability to hash a ``MultiIndex`` (:issue:`15224`) | |||
- ``Series/DataFrame.squeeze()`` have gained the ``axis`` parameter. (:issue:`15339`) | |||
- ``Series/DataFrame.squeeze()`` have gained the ``axis`` parameter. (:issue:`15339`)<<<<<<< f4edb053e17e51e8c2bed7c16755c4f7f3222117 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge residual :>
doc/source/whatsnew/v0.20.0.txt
Outdated
- ``DataFrame.to_excel()`` has a new ``freeze_panes`` parameter to turn on Freeze Panes when exporting to Excel (:issue:`15160`) | ||
- HTML table output skips ``colspan`` or ``rowspan`` attribute if equal to 1. (:issue:`15403`) | ||
- ``pd.TimedeltaIndex`` now has a custom datetick formatter specifically designed for nanosecond level precision (:issue:`8711`) | ||
- ``pd.types.concat.union_categoricals`` gained the ``ignore_ordered`` argument to allow ignoring the ordered attribute of unioned categoricals (:issue:`13410`). See the :ref:`categorical union docs <categorical.union>` for more information. | ||
- Using numerical names in ``MultiIndex`` causes less errors. (:issue:`12223`) (:issue:`15262`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
say instead about the bug report about output formatting with a MI under certain conditions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(:issue:`12223`, :issue:`15262`)
pandas/indexes/base.py
Outdated
@@ -2352,6 +2352,11 @@ def get_level_values(self, level): | |||
self._validate_index_level(level) | |||
return self | |||
|
|||
def _get_level_values(self, num): | |||
# Used to mirror implementation for MultiIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to add an actual doc-string
pandas/indexes/multi.py
Outdated
@@ -856,6 +856,7 @@ def _get_level_values(self, level): | |||
Parameters | |||
---------- | |||
level : int level | |||
copy : bool whether copy of results should be done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you adding this? this is a whole different ball game.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback What I needed _get_level_values()
to do is have the same behavior as the public get_level_values()
, with the assumption of the int argument, so the copy
argument makes that happen by doing the shallow copy there.
When I first looked at this, there was no _get_level_values()
, but that got introduced within the past 2 weeks, so I then had to make everything compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still its not clear why you would actually be making this change, it just adds too much complexity.
show an example of why you think you need it (or simply take it out)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback I'll try an alternate implementation and let you review that.
Codecov Report
@@ Coverage Diff @@
## master #15478 +/- ##
==========================================
+ Coverage 90.36% 90.36% +<.01%
==========================================
Files 135 135
Lines 49519 49522 +3
==========================================
+ Hits 44747 44750 +3
Misses 4772 4772
Continue to review full report at Codecov.
|
@jreback the last commit I did should have addressed the changes you requested, and we're all green, so let me know if there is more to do |
thanks @Dr-Irv |
@Dr-Irv Thanks! |
…n MultiIndex closes pandas-dev#12223 closes pandas-dev#15262 Author: Dr-Irv <[email protected]> Closes pandas-dev#15478 from Dr-Irv/Issue15262 and squashes the following commits: 15d8433 [Dr-Irv] Address jreback comments 10667a3 [Dr-Irv] Fix types for test 8935068 [Dr-Irv] resolve conflicts 385ca3e [Dr-Irv] BUG: GH pandas-dev#12223, GH pandas-dev#15262. Allow ints for names in MultiIndex
pandas/tests/frame/test_combine_concat.py::TestDataFrameConcatCommon::test_concat_numerical_names
git diff upstream/master | flake8 --diff
See discussion in #10461 . Now if we internally know we need to call
get_level_values
for a specific level number, there is an internal function for doing that, which should allow better support for when people create indices with names that are integers.