From 80bdd417ec9e9ce8fbff1c93a12d23a093831e6c Mon Sep 17 00:00:00 2001 From: Patrick Hoefler <61934744+phofl@users.noreply.github.com> Date: Thu, 20 Oct 2022 17:42:06 +0100 Subject: [PATCH] DEP: Enforce set_values and set_codes inplace and positional args deprecation (#49084) * DEP: Enforce set_values and set_codes inplace and positional args deprecation * Fix * Add whatsnew * Add test --- doc/source/whatsnew/v2.0.0.rst | 2 + pandas/core/indexes/multi.py | 50 +----- pandas/tests/indexes/multi/test_compat.py | 17 +- pandas/tests/indexes/multi/test_duplicates.py | 3 +- .../tests/indexes/multi/test_equivalence.py | 9 +- pandas/tests/indexes/multi/test_get_set.py | 164 +++--------------- pandas/tests/indexes/multi/test_integrity.py | 3 +- .../tests/indexing/multiindex/test_sorted.py | 10 +- 8 files changed, 38 insertions(+), 220 deletions(-) diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst index 41f16fd791d160..7c269e851e77f3 100644 --- a/doc/source/whatsnew/v2.0.0.rst +++ b/doc/source/whatsnew/v2.0.0.rst @@ -147,6 +147,8 @@ Removal of prior version deprecations/changes - Remove argument ``squeeze`` from :meth:`DataFrame.groupby` and :meth:`Series.groupby` (:issue:`32380`) - Removed ``keep_tz`` argument in :meth:`DatetimeIndex.to_series` (:issue:`29731`) - Remove arguments ``names`` and ``dtype`` from :meth:`Index.copy` and ``levels`` and ``codes`` from :meth:`MultiIndex.copy` (:issue:`35853`, :issue:`36685`) +- Remove argument ``inplace`` from :meth:`MultiIndex.set_levels` and :meth:`MultiIndex.set_codes` (:issue:`35626`) +- Disallow passing positional arguments to :meth:`MultiIndex.set_levels` and :meth:`MultiIndex.set_codes` (:issue:`41485`) - Removed argument ``try_cast`` from :meth:`DataFrame.mask`, :meth:`DataFrame.where`, :meth:`Series.mask` and :meth:`Series.where` (:issue:`38836`) - Disallow passing non-round floats to :class:`Timestamp` with ``unit="M"`` or ``unit="Y"`` (:issue:`47266`) - Removed the ``numeric_only`` keyword from :meth:`Categorical.min` and :meth:`Categorical.max` in favor of ``skipna`` (:issue:`48821`) diff --git a/pandas/core/indexes/multi.py b/pandas/core/indexes/multi.py index cf725019166eb7..59a0179f93c105 100644 --- a/pandas/core/indexes/multi.py +++ b/pandas/core/indexes/multi.py @@ -852,10 +852,7 @@ def _set_levels( self._reset_cache() - @deprecate_nonkeyword_arguments(version=None, allowed_args=["self", "levels"]) - def set_levels( - self, levels, level=None, inplace=None, verify_integrity: bool = True - ): + def set_levels(self, levels, *, level=None, verify_integrity: bool = True): """ Set new levels on MultiIndex. Defaults to returning new index. @@ -865,10 +862,6 @@ def set_levels( New level(s) to apply. level : int, level name, or sequence of int/level names (default None) Level(s) to set (None for all levels). - inplace : bool - If True, mutates in place. - - .. deprecated:: 1.2.0 verify_integrity : bool, default True If True, checks that levels and codes are compatible. @@ -940,30 +933,17 @@ def set_levels( >>> idx.set_levels([['a', 'b', 'c'], [1, 2, 3, 4]], level=[0, 1]).levels FrozenList([['a', 'b', 'c'], [1, 2, 3, 4]]) """ - if inplace is not None: - warnings.warn( - "inplace is deprecated and will be removed in a future version.", - FutureWarning, - stacklevel=find_stack_level(), - ) - else: - inplace = False if is_list_like(levels) and not isinstance(levels, Index): levels = list(levels) level, levels = _require_listlike(level, levels, "Levels") - - if inplace: - idx = self - else: - idx = self._view() + idx = self._view() idx._reset_identity() idx._set_levels( levels, level=level, validate=True, verify_integrity=verify_integrity ) - if not inplace: - return idx + return idx @property def nlevels(self) -> int: @@ -1041,8 +1021,7 @@ def _set_codes( self._reset_cache() - @deprecate_nonkeyword_arguments(version=None, allowed_args=["self", "codes"]) - def set_codes(self, codes, level=None, inplace=None, verify_integrity: bool = True): + def set_codes(self, codes, *, level=None, verify_integrity: bool = True): """ Set new codes on MultiIndex. Defaults to returning new index. @@ -1052,10 +1031,6 @@ def set_codes(self, codes, level=None, inplace=None, verify_integrity: bool = Tr New codes to apply. level : int, level name, or sequence of int/level names (default None) Level(s) to set (None for all levels). - inplace : bool - If True, mutates in place. - - .. deprecated:: 1.2.0 verify_integrity : bool, default True If True, checks that levels and codes are compatible. @@ -1101,25 +1076,12 @@ def set_codes(self, codes, level=None, inplace=None, verify_integrity: bool = Tr (1, 'two')], names=['foo', 'bar']) """ - if inplace is not None: - warnings.warn( - "inplace is deprecated and will be removed in a future version.", - FutureWarning, - stacklevel=find_stack_level(), - ) - else: - inplace = False level, codes = _require_listlike(level, codes, "Codes") - - if inplace: - idx = self - else: - idx = self._view() + idx = self._view() idx._reset_identity() idx._set_codes(codes, level=level, verify_integrity=verify_integrity) - if not inplace: - return idx + return idx # -------------------------------------------------------------------- # Index Internals diff --git a/pandas/tests/indexes/multi/test_compat.py b/pandas/tests/indexes/multi/test_compat.py index d50a44057bd26b..60608e0def7f53 100644 --- a/pandas/tests/indexes/multi/test_compat.py +++ b/pandas/tests/indexes/multi/test_compat.py @@ -59,18 +59,12 @@ def test_inplace_mutation_resets_values(): new_vals = mi1.set_levels(levels2).values tm.assert_almost_equal(vals2, new_vals) - # Non-inplace doesn't drop _values from _cache [implementation detail] + # Doesn't drop _values from _cache [implementation detail] tm.assert_almost_equal(mi1._cache["_values"], vals) # ...and values is still same too tm.assert_almost_equal(mi1.values, vals) - # Inplace should drop _values from _cache - with tm.assert_produces_warning(FutureWarning): - mi1.set_levels(levels2, inplace=True) - assert "_values" not in mi1._cache - tm.assert_almost_equal(mi1.values, vals2) - # Make sure label setting works too codes2 = [[0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0]] exp_values = np.empty((6,), dtype=object) @@ -84,15 +78,8 @@ def test_inplace_mutation_resets_values(): new_values = new_mi.values assert "_values" in new_mi._cache - # Not inplace shouldn't change + # Shouldn't change cache tm.assert_almost_equal(mi2._cache["_values"], vals2) # Should have correct values tm.assert_almost_equal(exp_values, new_values) - - # ...and again setting inplace should drop _values from _cache, etc - with tm.assert_produces_warning(FutureWarning): - mi2.set_codes(codes2, inplace=True) - assert "_values" not in mi2._cache - tm.assert_almost_equal(mi2.values, new_values) - assert "_values" in mi2._cache diff --git a/pandas/tests/indexes/multi/test_duplicates.py b/pandas/tests/indexes/multi/test_duplicates.py index ea21f284096998..509daff1262b4f 100644 --- a/pandas/tests/indexes/multi/test_duplicates.py +++ b/pandas/tests/indexes/multi/test_duplicates.py @@ -87,8 +87,7 @@ def test_duplicate_multiindex_codes(): mi = MultiIndex.from_arrays([["A", "A", "B", "B", "B"], [1, 2, 1, 2, 3]]) msg = r"Level values must be unique: \[[AB', ]+\] on level 0" with pytest.raises(ValueError, match=msg): - with tm.assert_produces_warning(FutureWarning): - mi.set_levels([["A", "B", "A", "A", "B"], [2, 1, 3, -2, 5]], inplace=True) + mi.set_levels([["A", "B", "A", "A", "B"], [2, 1, 3, -2, 5]]) @pytest.mark.parametrize("names", [["a", "b", "a"], [1, 1, 2], [1, "a", 1]]) diff --git a/pandas/tests/indexes/multi/test_equivalence.py b/pandas/tests/indexes/multi/test_equivalence.py index c6567b86f0da20..18ff73dcb22219 100644 --- a/pandas/tests/indexes/multi/test_equivalence.py +++ b/pandas/tests/indexes/multi/test_equivalence.py @@ -243,9 +243,6 @@ def test_is_(): assert mi.is_(mi2) assert not mi.is_(mi.set_names(["C", "D"])) - mi2 = mi.view() - mi2.set_names(["E", "F"], inplace=True) - assert mi.is_(mi2) # levels are inherent properties, they change identity mi3 = mi2.set_levels([list(range(10)), list(range(10))]) assert not mi3.is_(mi2) @@ -254,12 +251,10 @@ def test_is_(): mi4 = mi3.view() # GH 17464 - Remove duplicate MultiIndex levels - with tm.assert_produces_warning(FutureWarning): - mi4.set_levels([list(range(10)), list(range(10))], inplace=True) + mi4 = mi4.set_levels([list(range(10)), list(range(10))]) assert not mi4.is_(mi3) mi5 = mi.view() - with tm.assert_produces_warning(FutureWarning): - mi5.set_levels(mi5.levels, inplace=True) + mi5 = mi5.set_levels(mi5.levels) assert not mi5.is_(mi) diff --git a/pandas/tests/indexes/multi/test_get_set.py b/pandas/tests/indexes/multi/test_get_set.py index 3c64f7997b584a..4fff4ca961cf77 100644 --- a/pandas/tests/indexes/multi/test_get_set.py +++ b/pandas/tests/indexes/multi/test_get_set.py @@ -165,13 +165,6 @@ def test_set_levels(idx): assert_matching(ind2.levels, new_levels) assert_matching(idx.levels, levels) - # level changing [w/ mutation] - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_levels(new_levels, inplace=True) - assert inplace_return is None - assert_matching(ind2.levels, new_levels) - # level changing specific level [w/o mutation] ind2 = idx.set_levels(new_levels[0], level=0) assert_matching(ind2.levels, [new_levels[0], levels[1]]) @@ -186,52 +179,24 @@ def test_set_levels(idx): assert_matching(ind2.levels, new_levels) assert_matching(idx.levels, levels) - # level changing specific level [w/ mutation] - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_levels(new_levels[0], level=0, inplace=True) - assert inplace_return is None - assert_matching(ind2.levels, [new_levels[0], levels[1]]) - assert_matching(idx.levels, levels) - - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_levels(new_levels[1], level=1, inplace=True) - assert inplace_return is None - assert_matching(ind2.levels, [levels[0], new_levels[1]]) - assert_matching(idx.levels, levels) - - # level changing multiple levels [w/ mutation] - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_levels(new_levels, level=[0, 1], inplace=True) - assert inplace_return is None - assert_matching(ind2.levels, new_levels) - assert_matching(idx.levels, levels) - # illegal level changing should not change levels # GH 13754 original_index = idx.copy() - for inplace in [True, False]: - with pytest.raises(ValueError, match="^On"): - with tm.assert_produces_warning(FutureWarning): - idx.set_levels(["c"], level=0, inplace=inplace) - assert_matching(idx.levels, original_index.levels, check_dtype=True) + with pytest.raises(ValueError, match="^On"): + idx.set_levels(["c"], level=0) + assert_matching(idx.levels, original_index.levels, check_dtype=True) - with pytest.raises(ValueError, match="^On"): - with tm.assert_produces_warning(FutureWarning): - idx.set_codes([0, 1, 2, 3, 4, 5], level=0, inplace=inplace) - assert_matching(idx.codes, original_index.codes, check_dtype=True) + with pytest.raises(ValueError, match="^On"): + idx.set_codes([0, 1, 2, 3, 4, 5], level=0) + assert_matching(idx.codes, original_index.codes, check_dtype=True) - with pytest.raises(TypeError, match="^Levels"): - with tm.assert_produces_warning(FutureWarning): - idx.set_levels("c", level=0, inplace=inplace) - assert_matching(idx.levels, original_index.levels, check_dtype=True) + with pytest.raises(TypeError, match="^Levels"): + idx.set_levels("c", level=0) + assert_matching(idx.levels, original_index.levels, check_dtype=True) - with pytest.raises(TypeError, match="^Codes"): - with tm.assert_produces_warning(FutureWarning): - idx.set_codes(1, level=0, inplace=inplace) - assert_matching(idx.codes, original_index.codes, check_dtype=True) + with pytest.raises(TypeError, match="^Codes"): + idx.set_codes(1, level=0) + assert_matching(idx.codes, original_index.codes, check_dtype=True) def test_set_codes(idx): @@ -248,13 +213,6 @@ def test_set_codes(idx): assert_matching(ind2.codes, new_codes) assert_matching(idx.codes, codes) - # changing label w/ mutation - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_codes(new_codes, inplace=True) - assert inplace_return is None - assert_matching(ind2.codes, new_codes) - # codes changing specific level w/o mutation ind2 = idx.set_codes(new_codes[0], level=0) assert_matching(ind2.codes, [new_codes[0], codes[1]]) @@ -269,29 +227,6 @@ def test_set_codes(idx): assert_matching(ind2.codes, new_codes) assert_matching(idx.codes, codes) - # label changing specific level w/ mutation - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_codes(new_codes[0], level=0, inplace=True) - assert inplace_return is None - assert_matching(ind2.codes, [new_codes[0], codes[1]]) - assert_matching(idx.codes, codes) - - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_codes(new_codes[1], level=1, inplace=True) - assert inplace_return is None - assert_matching(ind2.codes, [codes[0], new_codes[1]]) - assert_matching(idx.codes, codes) - - # codes changing multiple levels [w/ mutation] - ind2 = idx.copy() - with tm.assert_produces_warning(FutureWarning): - inplace_return = ind2.set_codes(new_codes, level=[0, 1], inplace=True) - assert inplace_return is None - assert_matching(ind2.codes, new_codes) - assert_matching(idx.codes, codes) - # label changing for levels of different magnitude of categories ind = MultiIndex.from_tuples([(0, i) for i in range(130)]) new_codes = range(129, -1, -1) @@ -301,12 +236,6 @@ def test_set_codes(idx): result = ind.set_codes(codes=new_codes, level=1) assert result.equals(expected) - # [w/ mutation] - result = ind.copy() - with tm.assert_produces_warning(FutureWarning): - result.set_codes(codes=new_codes, level=1, inplace=True) - assert result.equals(expected) - def test_set_levels_codes_names_bad_input(idx): levels, codes = idx.levels, idx.codes @@ -433,49 +362,6 @@ def test_set_levels_with_iterable(): tm.assert_index_equal(result, expected) -@pytest.mark.parametrize("inplace", [True, False]) -def test_set_codes_inplace_deprecated(idx, inplace): - new_codes = idx.codes[1][::-1] - - with tm.assert_produces_warning(FutureWarning): - idx.set_codes(codes=new_codes, level=1, inplace=inplace) - - -@pytest.mark.parametrize("inplace", [True, False]) -def test_set_levels_inplace_deprecated(idx, inplace): - new_level = idx.levels[1].copy() - - with tm.assert_produces_warning(FutureWarning): - idx.set_levels(levels=new_level, level=1, inplace=inplace) - - -def test_set_levels_pos_args_deprecation(): - # https://github.com/pandas-dev/pandas/issues/41485 - idx = MultiIndex.from_tuples( - [ - (1, "one"), - (2, "one"), - (3, "one"), - ], - names=["foo", "bar"], - ) - msg = ( - r"In a future version of pandas all arguments of MultiIndex.set_levels except " - r"for the argument 'levels' will be keyword-only" - ) - with tm.assert_produces_warning(FutureWarning, match=msg): - result = idx.set_levels(["a", "b", "c"], 0) - expected = MultiIndex.from_tuples( - [ - ("a", "one"), - ("b", "one"), - ("c", "one"), - ], - names=["foo", "bar"], - ) - tm.assert_index_equal(result, expected) - - def test_set_empty_level(): # GH#48636 midx = MultiIndex.from_arrays([[]], names=["A"]) @@ -484,23 +370,17 @@ def test_set_empty_level(): tm.assert_index_equal(result, expected) -def test_set_codes_pos_args_depreciation(idx): +def test_set_levels_pos_args_removal(): # https://github.com/pandas-dev/pandas/issues/41485 - msg = ( - r"In a future version of pandas all arguments of MultiIndex.set_codes except " - r"for the argument 'codes' will be keyword-only" - ) - with tm.assert_produces_warning(FutureWarning, match=msg): - result = idx.set_codes([[0, 0, 1, 2, 3, 3], [0, 1, 0, 1, 0, 1]], [0, 1]) - expected = MultiIndex.from_tuples( + idx = MultiIndex.from_tuples( [ - ("foo", "one"), - ("foo", "two"), - ("bar", "one"), - ("baz", "two"), - ("qux", "one"), - ("qux", "two"), + (1, "one"), + (3, "one"), ], - names=["first", "second"], + names=["foo", "bar"], ) - tm.assert_index_equal(result, expected) + with pytest.raises(TypeError, match="positional arguments"): + idx.set_levels(["a", "b", "c"], 0) + + with pytest.raises(TypeError, match="positional arguments"): + idx.set_codes([[0, 1], [1, 0]], 0) diff --git a/pandas/tests/indexes/multi/test_integrity.py b/pandas/tests/indexes/multi/test_integrity.py index c35871745938b8..ef72f1f3ffde8a 100644 --- a/pandas/tests/indexes/multi/test_integrity.py +++ b/pandas/tests/indexes/multi/test_integrity.py @@ -226,8 +226,7 @@ def test_metadata_immutable(idx): def test_level_setting_resets_attributes(): ind = MultiIndex.from_arrays([["A", "A", "B", "B", "B"], [1, 2, 1, 2, 3]]) assert ind.is_monotonic_increasing - with tm.assert_produces_warning(FutureWarning): - ind.set_levels([["A", "B"], [1, 3, 2]], inplace=True) + ind = ind.set_levels([["A", "B"], [1, 3, 2]]) # if this fails, probably didn't reset the cache correctly. assert not ind.is_monotonic_increasing diff --git a/pandas/tests/indexing/multiindex/test_sorted.py b/pandas/tests/indexing/multiindex/test_sorted.py index 1d2fd6586337bf..ffea7fefe2d8d0 100644 --- a/pandas/tests/indexing/multiindex/test_sorted.py +++ b/pandas/tests/indexing/multiindex/test_sorted.py @@ -48,14 +48,8 @@ def test_frame_getitem_not_sorted2(self, key): df2 = df.set_index(["col1", "col2"]) df2_original = df2.copy() - with tm.assert_produces_warning(FutureWarning): - return_value = df2.index.set_levels( - ["b", "d", "a"], level="col1", inplace=True - ) - assert return_value is None - with tm.assert_produces_warning(FutureWarning): - return_value = df2.index.set_codes([0, 1, 0, 2], level="col1", inplace=True) - assert return_value is None + df2.index = df2.index.set_levels(["b", "d", "a"], level="col1") + df2.index = df2.index.set_codes([0, 1, 0, 2], level="col1") assert not df2.index.is_monotonic_increasing assert df2_original.index.equals(df2.index)