BUG: set_levels set illegal levels. (#14236)

`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.

Author: bkandel

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`)

doc/source/whatsnew/v0.19.1.txt

@@ -44,3 +44,4 @@ 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`)

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):
  """

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.