Skip to content

BUG: set_levels set illegal levels. #14290

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.19.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1576,5 +1576,6 @@ Bug Fixes

- Bug in ``eval()`` where the ``resolvers`` argument would not accept a list (:issue:`14095`)
- Bugs in ``stack``, ``get_dummies``, ``make_axis_dummies`` which don't preserve categorical dtypes in (multi)indexes (:issue:`13854`)
- ``PeridIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
- ``PeriodIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
- Bug in ``df.groupby`` where ``.median()`` returns arbitrary values if grouped dataframe contains empty bins (:issue:`13629`)
- Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`)
28 changes: 19 additions & 9 deletions pandas/indexes/multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,22 @@ def __new__(cls, levels=None, labels=None, sortorder=None, names=None,

return result

def _verify_integrity(self):
def _verify_integrity(self, new_labels=None, new_levels=None):
"""Raises ValueError if length of levels and labels don't match or any
label would exceed level bounds"""
label would exceed level bounds

Parameters
----------
new_labels : optional list
Labels to check for validity. Defaults to current labels.
new_levels : optional list
Levels to check for validity. Defaults to current levels.
"""
# NOTE: Currently does not check, among other things, that cached
# nlevels matches nor that sortorder matches actually sortorder.
labels, levels = self.labels, self.levels
labels = new_labels or self.labels
levels = new_levels or self.levels

if len(levels) != len(labels):
raise ValueError("Length of levels and labels must match. NOTE:"
" this index is in an inconsistent state.")
Expand Down Expand Up @@ -162,6 +172,9 @@ def _set_levels(self, levels, level=None, copy=False, validate=True,
new_levels[l] = _ensure_index(v, copy=copy)._shallow_copy()
new_levels = FrozenList(new_levels)

if verify_integrity:
self._verify_integrity(new_levels=new_levels)

names = self.names
self._levels = new_levels
if any(names):
Expand All @@ -170,9 +183,6 @@ def _set_levels(self, levels, level=None, copy=False, validate=True,
self._tuples = None
self._reset_cache()

if verify_integrity:
self._verify_integrity()

def set_levels(self, levels, level=None, inplace=False,
verify_integrity=True):
"""
Expand Down Expand Up @@ -268,13 +278,13 @@ def _set_labels(self, labels, level=None, copy=False, validate=True,
lab, lev, copy=copy)._shallow_copy()
new_labels = FrozenList(new_labels)

if verify_integrity:
self._verify_integrity(new_labels=new_labels)

self._labels = new_labels
self._tuples = None
self._reset_cache()

if verify_integrity:
self._verify_integrity()

def set_labels(self, labels, level=None, inplace=False,
verify_integrity=True):
"""
Expand Down
21 changes: 20 additions & 1 deletion pandas/tests/indexes/test_multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def assert_matching(actual, expected):
# as much as possible
self.assertEqual(len(actual), len(expected))
for act, exp in zip(actual, expected):
act = np.asarray(act)
act = np.asarray(act, dtype=np.object_)
exp = np.asarray(exp, dtype=np.object_)
tm.assert_numpy_array_equal(act, exp)

Expand Down Expand Up @@ -204,6 +204,25 @@ def assert_matching(actual, expected):
assert_matching(ind2.levels, new_levels)
assert_matching(self.index.levels, levels)

# illegal level changing should not change levels
# GH 13754
original_index = self.index.copy()
with assertRaisesRegexp(ValueError, "^On"):
self.index.set_levels(['c'], level=0, inplace=True)
assert_matching(self.index.levels, original_index.levels)

with assertRaisesRegexp(ValueError, "^On"):
self.index.set_labels([0, 1, 2, 3, 4, 5], level=0, inplace=True)
assert_matching(self.index.labels, original_index.labels)

with assertRaisesRegexp(TypeError, "^Levels"):
self.index.set_levels('c', level=0, inplace=True)
assert_matching(self.index.levels, original_index.levels)

with assertRaisesRegexp(TypeError, "^Labels"):
self.index.set_labels(1, level=0, inplace=True)
assert_matching(self.index.labels, original_index.labels)

def test_set_labels(self):
# side note - you probably wouldn't want to use levels and labels
# directly like this - but it is possible.
Expand Down