diff --git a/doc/source/whatsnew/v0.19.0.txt b/doc/source/whatsnew/v0.19.0.txt index 60847469aa02c..8e7e95c071ea4 100644 --- a/doc/source/whatsnew/v0.19.0.txt +++ b/doc/source/whatsnew/v0.19.0.txt @@ -1560,6 +1560,6 @@ Bug Fixes - Bug in ``.to_string()`` when called with an integer ``line_width`` and ``index=False`` raises an UnboundLocalError exception because ``idx`` referenced before assignment. - 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 ``Index.copy()`` where ``name`` parameter was ignored (:issue:`14302`) diff --git a/doc/source/whatsnew/v0.19.1.txt b/doc/source/whatsnew/v0.19.1.txt index 21b9fac6ffacf..92c7746d1c023 100644 --- a/doc/source/whatsnew/v0.19.1.txt +++ b/doc/source/whatsnew/v0.19.1.txt @@ -31,3 +31,4 @@ Performance Improvements Bug Fixes ~~~~~~~~~ - Bug in ``pd.concat`` where names of the ``keys`` were not propagated to the resulting ``MultiIndex`` (:issue:`14252`) +- Bug in ``MultiIndex.set_levels`` where illegal level values were still set after raising an error (:issue:`13754`) diff --git a/pandas/indexes/multi.py b/pandas/indexes/multi.py index 1ab5dbb737739..0c465da24a17e 100644 --- a/pandas/indexes/multi.py +++ b/pandas/indexes/multi.py @@ -116,12 +116,27 @@ def __new__(cls, levels=None, labels=None, sortorder=None, names=None, return result - def _verify_integrity(self): - """Raises ValueError if length of levels and labels don't match or any - label would exceed level bounds""" + def _verify_integrity(self, labels=None, levels=None): + """ + + Parameters + ---------- + labels : optional list + Labels to check for validity. Defaults to current labels. + levels : optional list + Levels to check for validity. Defaults to current levels. + + Raises + ------ + ValueError + * if length of levels and labels don't match or any label would + exceed level bounds + """ # 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 = labels or self.labels + levels = 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.") @@ -162,6 +177,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(levels=new_levels) + names = self.names self._levels = new_levels if any(names): @@ -170,9 +188,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): """ @@ -268,13 +283,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(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): """ diff --git a/pandas/tests/indexes/test_multi.py b/pandas/tests/indexes/test_multi.py index cd9ce0102ca1e..fdc5a2eaec812 100644 --- a/pandas/tests/indexes/test_multi.py +++ b/pandas/tests/indexes/test_multi.py @@ -149,14 +149,14 @@ def test_set_levels(self): levels = self.index.levels new_levels = [[lev + 'a' for lev in level] for level in levels] - def assert_matching(actual, expected): + def assert_matching(actual, expected, check_dtype=False): # avoid specifying internal representation # as much as possible self.assertEqual(len(actual), len(expected)) for act, exp in zip(actual, expected): act = np.asarray(act) - exp = np.asarray(exp, dtype=np.object_) - tm.assert_numpy_array_equal(act, exp) + exp = np.asarray(exp) + tm.assert_numpy_array_equal(act, exp, check_dtype=check_dtype) # level changing [w/o mutation] ind2 = self.index.set_levels(new_levels) @@ -204,6 +204,31 @@ 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() + for inplace in [True, False]: + with assertRaisesRegexp(ValueError, "^On"): + self.index.set_levels(['c'], level=0, inplace=inplace) + assert_matching(self.index.levels, original_index.levels, + check_dtype=True) + + with assertRaisesRegexp(ValueError, "^On"): + self.index.set_labels([0, 1, 2, 3, 4, 5], level=0, + inplace=inplace) + assert_matching(self.index.labels, original_index.labels, + check_dtype=True) + + with assertRaisesRegexp(TypeError, "^Levels"): + self.index.set_levels('c', level=0, inplace=inplace) + assert_matching(self.index.levels, original_index.levels, + check_dtype=True) + + with assertRaisesRegexp(TypeError, "^Labels"): + self.index.set_labels(1, level=0, inplace=inplace) + assert_matching(self.index.labels, original_index.labels, + check_dtype=True) + def test_set_labels(self): # side note - you probably wouldn't want to use levels and labels # directly like this - but it is possible.