Skip to content

Commit 39b7bde

Browse files
author
Ben Kandel
committed
addressed comments
1 parent 005ab51 commit 39b7bde

File tree

4 files changed

+22
-20
lines changed

4 files changed

+22
-20
lines changed

doc/source/whatsnew/v0.19.0.txt

-1
Original file line numberDiff line numberDiff line change
@@ -1563,4 +1563,3 @@ Bug Fixes
15631563
- ``PeriodIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
15641564
- Bug in ``df.groupby`` where ``.median()`` returns arbitrary values if grouped dataframe contains empty bins (:issue:`13629`)
15651565
- Bug in ``Index.copy()`` where ``name`` parameter was ignored (:issue:`14302`)
1566-
- Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`)

doc/source/whatsnew/v0.19.1.txt

+1
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ Performance Improvements
3131
Bug Fixes
3232
~~~~~~~~~
3333
- Bug in ``pd.concat`` where names of the ``keys`` were not propagated to the resulting ``MultiIndex`` (:issue:`14252`)
34+
- Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`)

pandas/indexes/multi.py

+14-9
Original file line numberDiff line numberDiff line change
@@ -116,21 +116,26 @@ def __new__(cls, levels=None, labels=None, sortorder=None, names=None,
116116

117117
return result
118118

119-
def _verify_integrity(self, new_labels=None, new_levels=None):
120-
"""Raises ValueError if length of levels and labels don't match or any
121-
label would exceed level bounds
119+
def _verify_integrity(self, labels=None, levels=None):
120+
"""
122121
123122
Parameters
124123
----------
125-
new_labels : optional list
124+
labels : optional list
126125
Labels to check for validity. Defaults to current labels.
127-
new_levels : optional list
126+
levels : optional list
128127
Levels to check for validity. Defaults to current levels.
128+
129+
Raises
130+
------
131+
ValueError
132+
* if length of levels and labels don't match or any label would
133+
exceed level bounds
129134
"""
130135
# NOTE: Currently does not check, among other things, that cached
131136
# nlevels matches nor that sortorder matches actually sortorder.
132-
labels = new_labels or self.labels
133-
levels = new_levels or self.levels
137+
labels = labels or self.labels
138+
levels = levels or self.levels
134139

135140
if len(levels) != len(labels):
136141
raise ValueError("Length of levels and labels must match. NOTE:"
@@ -173,7 +178,7 @@ def _set_levels(self, levels, level=None, copy=False, validate=True,
173178
new_levels = FrozenList(new_levels)
174179

175180
if verify_integrity:
176-
self._verify_integrity(new_levels=new_levels)
181+
self._verify_integrity(levels=new_levels)
177182

178183
names = self.names
179184
self._levels = new_levels
@@ -279,7 +284,7 @@ def _set_labels(self, labels, level=None, copy=False, validate=True,
279284
new_labels = FrozenList(new_labels)
280285

281286
if verify_integrity:
282-
self._verify_integrity(new_labels=new_labels)
287+
self._verify_integrity(labels=new_labels)
283288

284289
self._labels = new_labels
285290
self._tuples = None

pandas/tests/indexes/test_multi.py

+7-10
Original file line numberDiff line numberDiff line change
@@ -149,17 +149,14 @@ def test_set_levels(self):
149149
levels = self.index.levels
150150
new_levels = [[lev + 'a' for lev in level] for level in levels]
151151

152-
def assert_matching(actual, expected, coerce_to_obj=True):
152+
def assert_matching(actual, expected, check_dtype=False):
153153
# avoid specifying internal representation
154154
# as much as possible
155155
self.assertEqual(len(actual), len(expected))
156156
for act, exp in zip(actual, expected):
157157
act = np.asarray(act)
158-
if coerce_to_obj:
159-
exp = np.asarray(exp, dtype=np.object_)
160-
else:
161-
exp = np.asarray(exp)
162-
tm.assert_numpy_array_equal(act, exp)
158+
exp = np.asarray(exp)
159+
tm.assert_numpy_array_equal(act, exp, check_dtype=check_dtype)
163160

164161
# level changing [w/o mutation]
165162
ind2 = self.index.set_levels(new_levels)
@@ -214,23 +211,23 @@ def assert_matching(actual, expected, coerce_to_obj=True):
214211
with assertRaisesRegexp(ValueError, "^On"):
215212
self.index.set_levels(['c'], level=0, inplace=inplace)
216213
assert_matching(self.index.levels, original_index.levels,
217-
coerce_to_obj=False)
214+
check_dtype=True)
218215

219216
with assertRaisesRegexp(ValueError, "^On"):
220217
self.index.set_labels([0, 1, 2, 3, 4, 5], level=0,
221218
inplace=inplace)
222219
assert_matching(self.index.labels, original_index.labels,
223-
coerce_to_obj=False)
220+
check_dtype=True)
224221

225222
with assertRaisesRegexp(TypeError, "^Levels"):
226223
self.index.set_levels('c', level=0, inplace=inplace)
227224
assert_matching(self.index.levels, original_index.levels,
228-
coerce_to_obj=False)
225+
check_dtype=True)
229226

230227
with assertRaisesRegexp(TypeError, "^Labels"):
231228
self.index.set_labels(1, level=0, inplace=inplace)
232229
assert_matching(self.index.labels, original_index.labels,
233-
coerce_to_obj=False)
230+
check_dtype=True)
234231

235232
def test_set_labels(self):
236233
# side note - you probably wouldn't want to use levels and labels

0 commit comments

Comments
 (0)