Skip to content

Commit 2cb6d8c

Browse files
author
Ben Kandel
committed
BUG: set_levels set illegal levels.
`MultiIndex.set_levels`, when passed in illegal level values, raises an error. When `inplace=True`, though, the illegal level values are still accepted. This commit fixes that behavior by checking that the proposed level values are legal before setting them. added tests and docs. lint added tests. force build force build force build force build for osx
1 parent d9e51fe commit 2cb6d8c

File tree

3 files changed

+41
-11
lines changed

3 files changed

+41
-11
lines changed

doc/source/whatsnew/v0.19.0.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -1576,5 +1576,6 @@ Bug Fixes
15761576

15771577
- Bug in ``eval()`` where the ``resolvers`` argument would not accept a list (:issue:`14095`)
15781578
- Bugs in ``stack``, ``get_dummies``, ``make_axis_dummies`` which don't preserve categorical dtypes in (multi)indexes (:issue:`13854`)
1579-
- ``PeridIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
1579+
- ``PeriodIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
15801580
- Bug in ``df.groupby`` where ``.median()`` returns arbitrary values if grouped dataframe contains empty bins (:issue:`13629`)
1581+
- Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`)

pandas/indexes/multi.py

+19-9
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,22 @@ def __new__(cls, levels=None, labels=None, sortorder=None, names=None,
116116

117117
return result
118118

119-
def _verify_integrity(self):
119+
def _verify_integrity(self, new_labels=None, new_levels=None):
120120
"""Raises ValueError if length of levels and labels don't match or any
121-
label would exceed level bounds"""
121+
label would exceed level bounds
122+
123+
Parameters
124+
----------
125+
new_labels : optional list
126+
Labels to check for validity. Defaults to current labels.
127+
new_levels : optional list
128+
Levels to check for validity. Defaults to current levels.
129+
"""
122130
# NOTE: Currently does not check, among other things, that cached
123131
# nlevels matches nor that sortorder matches actually sortorder.
124-
labels, levels = self.labels, self.levels
132+
labels = new_labels or self.labels
133+
levels = new_levels or self.levels
134+
125135
if len(levels) != len(labels):
126136
raise ValueError("Length of levels and labels must match. NOTE:"
127137
" this index is in an inconsistent state.")
@@ -162,6 +172,9 @@ def _set_levels(self, levels, level=None, copy=False, validate=True,
162172
new_levels[l] = _ensure_index(v, copy=copy)._shallow_copy()
163173
new_levels = FrozenList(new_levels)
164174

175+
if verify_integrity:
176+
self._verify_integrity(new_levels=new_levels)
177+
165178
names = self.names
166179
self._levels = new_levels
167180
if any(names):
@@ -170,9 +183,6 @@ def _set_levels(self, levels, level=None, copy=False, validate=True,
170183
self._tuples = None
171184
self._reset_cache()
172185

173-
if verify_integrity:
174-
self._verify_integrity()
175-
176186
def set_levels(self, levels, level=None, inplace=False,
177187
verify_integrity=True):
178188
"""
@@ -268,13 +278,13 @@ def _set_labels(self, labels, level=None, copy=False, validate=True,
268278
lab, lev, copy=copy)._shallow_copy()
269279
new_labels = FrozenList(new_labels)
270280

281+
if verify_integrity:
282+
self._verify_integrity(new_labels=new_labels)
283+
271284
self._labels = new_labels
272285
self._tuples = None
273286
self._reset_cache()
274287

275-
if verify_integrity:
276-
self._verify_integrity()
277-
278288
def set_labels(self, labels, level=None, inplace=False,
279289
verify_integrity=True):
280290
"""

pandas/tests/indexes/test_multi.py

+20-1
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def assert_matching(actual, expected):
154154
# as much as possible
155155
self.assertEqual(len(actual), len(expected))
156156
for act, exp in zip(actual, expected):
157-
act = np.asarray(act)
157+
act = np.asarray(act, dtype=np.object_)
158158
exp = np.asarray(exp, dtype=np.object_)
159159
tm.assert_numpy_array_equal(act, exp)
160160

@@ -204,6 +204,25 @@ def assert_matching(actual, expected):
204204
assert_matching(ind2.levels, new_levels)
205205
assert_matching(self.index.levels, levels)
206206

207+
# illegal level changing should not change levels
208+
# GH 13754
209+
original_index = self.index.copy()
210+
with assertRaisesRegexp(ValueError, "^On"):
211+
self.index.set_levels(['c'], level=0, inplace=True)
212+
assert_matching(self.index.levels, original_index.levels)
213+
214+
with assertRaisesRegexp(ValueError, "^On"):
215+
self.index.set_labels([0, 1, 2, 3, 4, 5], level=0, inplace=True)
216+
assert_matching(self.index.labels, original_index.labels)
217+
218+
with assertRaisesRegexp(TypeError, "^Levels"):
219+
self.index.set_levels('c', level=0, inplace=True)
220+
assert_matching(self.index.levels, original_index.levels)
221+
222+
with assertRaisesRegexp(TypeError, "^Labels"):
223+
self.index.set_labels(1, level=0, inplace=True)
224+
assert_matching(self.index.labels, original_index.labels)
225+
207226
def test_set_labels(self):
208227
# side note - you probably wouldn't want to use levels and labels
209228
# directly like this - but it is possible.

0 commit comments

Comments
 (0)