BUG: MultiIndex.set_levels with emtpy levels #16987
Conversation
| @@ -428,6 +428,50 @@ def test_xs_multiindex(self): | |||
| expected.columns = expected.columns.droplevel('lvl1') | |||
| tm.assert_frame_equal(result, expected) | |||
|
|
|||
| def test_set_level_checkall(self): | |||
|
|
|||
|
Hello @leej3! Thanks for updating the PR.
Comment last updated on September 07, 2017 at 23:02 Hours UTC |
|
I'll try pass some more tests for this tomorrow. |
| @@ -107,6 +107,13 @@ Other API Changes | |||
| - Compression defaults in HDF stores now follow pytable standards. Default is no compression and if ``complib`` is missing and ``complevel`` > 0 ``zlib`` is used (:issue:`15943`) | |||
| - ``Index.get_indexer_non_unique()`` now returns a ndarray indexer rather than an ``Index``; this is consistent with ``Index.get_indexer()`` (:issue:`16819`) | |||
| - Removed the ``@slow`` decorator from ``pandas.util.testing``, which caused issues for some downstream packages' test suites. Use ``@pytest.mark.slow`` instead, which achieves the same thing (:issue:`16850`) | |||
| - func:`pandas.MultiIndex.set_levels()` now allows the setting of empty levels and, when `level` is a list-like object, each element of levels is confirmed to be list-like (:issue:`16147`) | |||
There was a problem hiding this comment.
You can put this under bug fixes. And I think it'll be
:meth:`pandas.MultiIndex.set_levels` (but `:func:` may do the same thing)
Finally, I think you can remove the "when level is a list-like object, each element of levels is confirmed to be list-like" section. That's more of an internal thing I think, that we're checking all the items in levels instead of just the first.
There was a problem hiding this comment.
cool, thanks. I'll sort this stuff out when I pass some of the other tests. I haven't accounted for all cases. Looks like it is indeed linked with #14823. I tried merging my code with that PR but still fail some tests, notably when levels consists of the Index class. I won't have much time over the next 2 weeks but we shall see.
| "list-like object containing list-like objects") | ||
| if not levels_list_like: | ||
| raise TypeError(levels_error) | ||
| elif not level_list_like: |
There was a problem hiding this comment.
This can just be else I think?
| levels_error = ("`levels` and `level` are incompatible. " | ||
| "When `level` is a scalar, `levels` must be a " | ||
| " list-like object of scalars.") | ||
| if levels_list_like: |
There was a problem hiding this comment.
I think this will behave incorrectly for setting index.set_index(levels=[], level=0). That should work, just setting that level's levels to []. In other words, this test should pass
def test_thing(self):
index = pd.MultiIndex(levels=[['a', 'b'], [1, 2]], labels=[[], []])
result = index.set_levels([], level=0)
expected = pd.MultiIndex(levels=[[], [1, 2]], labels=[[], []])
tm.assert_index_equal(result, expected)lmk if that makes sense. I may be incorrect.
| names=[u'foo', u'bar']) | ||
| tm.assert_index_equal(result, expected) | ||
|
|
||
| result = idx.set_levels(['a', 'b'], level=0) |
There was a problem hiding this comment.
Now that we have these tests passing, it may be good to split these up into multiple tests so it's easier to review.
jreback
changed the title
doc/source/whatsnew/v0.21.0.txt
Outdated
| - func:`pandas.MultiIndex.set_levels()` now allows the setting of empty levels and, when `level` is a list-like object, each element of levels is confirmed to be list-like (:issue:`16147`) | ||
|
|
||
| .. _whatsnew_0210.api: | ||
|
|
There was a problem hiding this comment.
remove this Other API Changes code as this has already been changed in master.
| raise TypeError("Levels must be list-like") | ||
| if is_list_like(levels[0]): | ||
| raise TypeError("Levels must be list-like") | ||
|
|
| raise TypeError("Levels must be list-like") | ||
|
|
||
| level_list_like = level is None or is_list_like(level) | ||
| levels_list_like = (is_list_like(levels) and |
There was a problem hiding this comment.
further some of this validation is duplicative of _set_levels. So rather add any additional validation there directly.
There was a problem hiding this comment.
I've had a try at this. Still some work to do on it
|
can you rebase and update |
leej3
force-pushed
the
issue_#16147_fix
branch
from
4d1655e to
330cf77
Compare
|
@leej3 can you rebase and update |
|
can you rebase / update |
|
closing as stale, if you want to work on this, pls ping. |
Helped by @TomAugspurger for this one. May he can take a review the changes too.
Thanks.