From 496fa9a15d4b4ccd02c09918a8b2d52f1c2e925a Mon Sep 17 00:00:00 2001 From: Patrick Hoefler <61934744+phofl@users.noreply.github.com> Date: Mon, 27 Nov 2023 22:37:14 +0100 Subject: [PATCH 1/2] CoW: Avoid warning if temporary object is a copy --- pandas/core/frame.py | 2 +- pandas/core/generic.py | 54 ++++++++++++++++--- pandas/core/series.py | 2 +- pandas/tests/copy_view/test_clip.py | 4 +- pandas/tests/copy_view/test_interp_fillna.py | 8 +-- pandas/tests/copy_view/test_methods.py | 8 +-- pandas/tests/copy_view/test_replace.py | 4 +- .../multiindex/test_chaining_and_caching.py | 4 +- 8 files changed, 62 insertions(+), 24 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 5d05983529fba..0e52c025ed1d7 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -8854,7 +8854,7 @@ def update( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif not PYPY and not using_copy_on_write() and self._is_view_after_cow_rules(): if sys.getrefcount(self) <= REF_COUNT: warnings.warn( _chained_assignment_warning_method_msg, diff --git a/pandas/core/generic.py b/pandas/core/generic.py index 56001fabfdc9d..cdef004803cfb 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -651,6 +651,12 @@ def _get_cleaned_column_resolvers(self) -> dict[Hashable, Series]: def _info_axis(self) -> Index: return getattr(self, self._info_axis_name) + def _is_view_after_cow_rules(self): + # Only to be used in cases of chained assignment checks, this is a + # simplified check that assumes that either the whole object is a view + # or a copy + return self._mgr.blocks[0].refs.has_reference() + @property def shape(self) -> tuple[int, ...]: """ @@ -7193,7 +7199,11 @@ def fillna( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and _check_cacher(self): @@ -7475,7 +7485,11 @@ def ffill( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and _check_cacher(self): @@ -7658,7 +7672,11 @@ def bfill( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and _check_cacher(self): @@ -7824,7 +7842,11 @@ def replace( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and _check_cacher(self): @@ -8265,7 +8287,11 @@ def interpolate( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and _check_cacher(self): @@ -8903,7 +8929,11 @@ def clip( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and hasattr(self, "_cacher"): @@ -10815,7 +10845,11 @@ def where( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and hasattr(self, "_cacher"): @@ -10894,7 +10928,11 @@ def mask( ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif ( + not PYPY + and not using_copy_on_write() + and self._is_view_after_cow_rules() + ): ctr = sys.getrefcount(self) ref_count = REF_COUNT if isinstance(self, ABCSeries) and hasattr(self, "_cacher"): diff --git a/pandas/core/series.py b/pandas/core/series.py index 3f7ac75e623a2..4d62f7c9ca14e 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -3554,7 +3554,7 @@ def update(self, other: Series | Sequence | Mapping) -> None: ChainedAssignmentError, stacklevel=2, ) - elif not PYPY and not using_copy_on_write(): + elif not PYPY and not using_copy_on_write() and self._is_view_after_cow_rules(): ctr = sys.getrefcount(self) ref_count = REF_COUNT if _check_cacher(self): diff --git a/pandas/tests/copy_view/test_clip.py b/pandas/tests/copy_view/test_clip.py index 13ddd479c7c67..9be9ba6f144c4 100644 --- a/pandas/tests/copy_view/test_clip.py +++ b/pandas/tests/copy_view/test_clip.py @@ -88,10 +88,10 @@ def test_clip_chained_inplace(using_copy_on_write): with tm.assert_produces_warning(FutureWarning, match="inplace method"): df["a"].clip(1, 2, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[["a"]].clip(1, 2, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[df["a"] > 1].clip(1, 2, inplace=True) diff --git a/pandas/tests/copy_view/test_interp_fillna.py b/pandas/tests/copy_view/test_interp_fillna.py index cdb06de90a568..5091fd9aed8a4 100644 --- a/pandas/tests/copy_view/test_interp_fillna.py +++ b/pandas/tests/copy_view/test_interp_fillna.py @@ -366,11 +366,11 @@ def test_fillna_chained_assignment(using_copy_on_write): df[["a"]].fillna(100, inplace=True) tm.assert_frame_equal(df, df_orig) else: - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[["a"]].fillna(100, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[df.a > 5].fillna(100, inplace=True) @@ -394,10 +394,10 @@ def test_interpolate_chained_assignment(using_copy_on_write, func): with tm.assert_produces_warning(FutureWarning, match="inplace method"): getattr(df["a"], func)(inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): getattr(df[["a"]], func)(inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): getattr(df[df["a"] > 1], func)(inplace=True) diff --git a/pandas/tests/copy_view/test_methods.py b/pandas/tests/copy_view/test_methods.py index 806842dcab57a..45e3412a12f30 100644 --- a/pandas/tests/copy_view/test_methods.py +++ b/pandas/tests/copy_view/test_methods.py @@ -1555,11 +1555,11 @@ def test_chained_where_mask(using_copy_on_write, func): with tm.assert_produces_warning(FutureWarning, match="inplace method"): getattr(df["a"], func)(df["a"] > 2, 5, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): getattr(df[["a"]], func)(df["a"] > 2, 5, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): getattr(df[df["a"] > 1], func)(df["a"] > 2, 5, inplace=True) @@ -1829,11 +1829,11 @@ def test_update_chained_assignment(using_copy_on_write): with tm.assert_produces_warning(FutureWarning, match="inplace method"): df["a"].update(ser2) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[["a"]].update(ser2.to_frame()) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[df["a"] > 1].update(ser2.to_frame()) diff --git a/pandas/tests/copy_view/test_replace.py b/pandas/tests/copy_view/test_replace.py index d11a2893becdc..f9d07df3b7df2 100644 --- a/pandas/tests/copy_view/test_replace.py +++ b/pandas/tests/copy_view/test_replace.py @@ -397,11 +397,11 @@ def test_replace_chained_assignment(using_copy_on_write): df[["a"]].replace(1, 100, inplace=True) tm.assert_frame_equal(df, df_orig) else: - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[["a"]].replace(1, 100, inplace=True) - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): with option_context("mode.chained_assignment", None): df[df.a > 5].replace(1, 100, inplace=True) diff --git a/pandas/tests/indexing/multiindex/test_chaining_and_caching.py b/pandas/tests/indexing/multiindex/test_chaining_and_caching.py index c71b077a5e242..02b37aea78d3b 100644 --- a/pandas/tests/indexing/multiindex/test_chaining_and_caching.py +++ b/pandas/tests/indexing/multiindex/test_chaining_and_caching.py @@ -34,12 +34,12 @@ def test_detect_chained_assignment(using_copy_on_write, warn_copy_on_write): with tm.raises_chained_assignment_error(): zed["eyes"]["right"].fillna(value=555, inplace=True) elif warn_copy_on_write: - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): zed["eyes"]["right"].fillna(value=555, inplace=True) else: msg = "A value is trying to be set on a copy of a slice from a DataFrame" with pytest.raises(SettingWithCopyError, match=msg): - with tm.assert_produces_warning(FutureWarning, match="inplace method"): + with tm.assert_produces_warning(None): zed["eyes"]["right"].fillna(value=555, inplace=True) From 93df0353e20dd879d3a1464543785cf84320101f Mon Sep 17 00:00:00 2001 From: Patrick Hoefler <61934744+phofl@users.noreply.github.com> Date: Mon, 27 Nov 2023 23:44:20 +0100 Subject: [PATCH 2/2] Fixup --- pandas/core/generic.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/core/generic.py b/pandas/core/generic.py index cdef004803cfb..9a78f608c7b36 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -655,7 +655,9 @@ def _is_view_after_cow_rules(self): # Only to be used in cases of chained assignment checks, this is a # simplified check that assumes that either the whole object is a view # or a copy - return self._mgr.blocks[0].refs.has_reference() + if len(self._mgr.blocks) == 0: # type: ignore[union-attr] + return False + return self._mgr.blocks[0].refs.has_reference() # type: ignore[union-attr] @property def shape(self) -> tuple[int, ...]: