BUG: GH #12223, GH #15262. Allow ints for names in MultiIndex #15478
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. |
jreback
added
Bug
MultiIndex
Output-Formatting
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 | |||
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.
say instead about the bug report about output formatting with a MI under certain conditions.
There was a problem hiding this comment.
(: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.
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.
why are you adding this? this is a whole different ball game.
There was a problem hiding this comment.
@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.
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.
@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 <irv@princeton.com> 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_namesgit diff upstream/master | flake8 --diffSee discussion in #10461 . Now if we internally know we need to call
get_level_valuesfor 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.