From 087994ca77d22fefb95d95ed27a9ab966488d4b7 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Wed, 2 Nov 2022 10:43:12 +0100 Subject: [PATCH 1/4] BUG / CoW: also return new object in case of null slice for both rows and columsn (.(i)loc[:, :]) --- pandas/core/indexing.py | 8 ++++ pandas/tests/copy_view/test_indexing.py | 62 +++++++++++++++++++++++++ pandas/tests/indexing/test_loc.py | 5 +- 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/pandas/core/indexing.py b/pandas/core/indexing.py index 70b0ee4b1d354..9b5747aec2d44 100644 --- a/pandas/core/indexing.py +++ b/pandas/core/indexing.py @@ -13,6 +13,8 @@ import numpy as np +from pandas._config import get_option + from pandas._libs.indexing import NDFrameIndexerBase from pandas._libs.lib import item_from_zerodim from pandas._typing import ( @@ -934,6 +936,12 @@ def _getitem_tuple_same_dim(self, tup: tuple): # be handled by the _getitem_lowerdim call above. assert retval.ndim == self.ndim + if retval is self.obj and ( + get_option("mode.copy_on_write") + and get_option("mode.data_manager") == "block" + ): + retval = retval.copy(deep=False) + return retval @final diff --git a/pandas/tests/copy_view/test_indexing.py b/pandas/tests/copy_view/test_indexing.py index b8028fd28f8f8..b226b61221686 100644 --- a/pandas/tests/copy_view/test_indexing.py +++ b/pandas/tests/copy_view/test_indexing.py @@ -612,6 +612,68 @@ def test_subset_chained_single_block_row(using_copy_on_write, using_array_manage assert subset.iloc[0] == 0 +@pytest.mark.parametrize( + "method", + [ + lambda df: df[:], + lambda df: df.loc[:, :], + lambda df: df.loc[:], + lambda df: df.iloc[:, :], + lambda df: df.iloc[:], + ], + ids=["getitem", "loc", "loc-rows", "iloc", "iloc-rows"], +) +def test_null_slice(request, method, using_copy_on_write): + # Case: also all variants of indexing with a null slice (:) should return + # new objects to ensure we correctly use CoW for the results + df = DataFrame({"a": [1, 2, 3], "b": [4, 5, 6], "c": [7, 8, 9]}) + df_orig = df.copy() + + df2 = method(df) + + # with CoW, we always return new objects + if using_copy_on_write: + assert df2 is not df + else: + if request.node.callspec.id in ("loc", "iloc"): + assert df2 is df + else: + assert df2 is not df + + # and those trigger CoW when mutated + df2.iloc[0, 0] = 0 + if using_copy_on_write: + tm.assert_frame_equal(df, df_orig) + else: + assert df.iloc[0, 0] == 0 + + +@pytest.mark.parametrize( + "method", + [ + lambda s: s[:], + lambda s: s.loc[:], + lambda s: s.iloc[:], + ], + ids=["getitem", "loc", "iloc"], +) +def test_null_slice_series(request, method, using_copy_on_write): + s = Series([1, 2, 3], index=["a", "b", "c"]) + s_orig = s.copy() + + s2 = method(s) + + # with CoW, we always return new objects (also for non-CoW this is the case) + assert s2 is not s + + # and those trigger CoW when mutated + s2.iloc[0] = 0 + if using_copy_on_write: + tm.assert_series_equal(s, s_orig) + else: + assert s.iloc[0] == 0 + + # TODO add more tests modifying the parent diff --git a/pandas/tests/indexing/test_loc.py b/pandas/tests/indexing/test_loc.py index d462ef534e02f..17be1c7d97a1d 100644 --- a/pandas/tests/indexing/test_loc.py +++ b/pandas/tests/indexing/test_loc.py @@ -1115,7 +1115,10 @@ def test_identity_slice_returns_new_object( assert (sliced_df["a"] == 4).all() # These should not return copies - assert original_df is original_df.loc[:, :] + if using_copy_on_write: + assert original_df is not original_df.loc[:, :] + else: + assert original_df is original_df.loc[:, :] df = DataFrame(np.random.randn(10, 4)) assert df[0] is df.loc[:, 0] From ca7d82752111cc870ecbfd0b7358e54fb111cd51 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Mon, 28 Nov 2022 13:05:51 +0100 Subject: [PATCH 2/4] also return new object for default non-CoW mode --- pandas/core/indexing.py | 9 +++------ pandas/tests/copy_view/test_indexing.py | 12 +++--------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/pandas/core/indexing.py b/pandas/core/indexing.py index fec04bb2b9a97..070ec7c7a2e4a 100644 --- a/pandas/core/indexing.py +++ b/pandas/core/indexing.py @@ -13,8 +13,6 @@ import numpy as np -from pandas._config import get_option - from pandas._libs.indexing import NDFrameIndexerBase from pandas._libs.lib import item_from_zerodim from pandas._typing import ( @@ -936,10 +934,9 @@ def _getitem_tuple_same_dim(self, tup: tuple): # be handled by the _getitem_lowerdim call above. assert retval.ndim == self.ndim - if retval is self.obj and ( - get_option("mode.copy_on_write") - and get_option("mode.data_manager") == "block" - ): + if retval is self.obj: + # if all axes were a null slice (`df.loc[:, :]`), ensure we still + # return a new object (https://github.com/pandas-dev/pandas/pull/49469) retval = retval.copy(deep=False) return retval diff --git a/pandas/tests/copy_view/test_indexing.py b/pandas/tests/copy_view/test_indexing.py index b226b61221686..5ebbbcfb5a301 100644 --- a/pandas/tests/copy_view/test_indexing.py +++ b/pandas/tests/copy_view/test_indexing.py @@ -631,14 +631,8 @@ def test_null_slice(request, method, using_copy_on_write): df2 = method(df) - # with CoW, we always return new objects - if using_copy_on_write: - assert df2 is not df - else: - if request.node.callspec.id in ("loc", "iloc"): - assert df2 is df - else: - assert df2 is not df + # we always return new objects (shallow copy), regardless of CoW or not + assert df2 is not df # and those trigger CoW when mutated df2.iloc[0, 0] = 0 @@ -663,7 +657,7 @@ def test_null_slice_series(request, method, using_copy_on_write): s2 = method(s) - # with CoW, we always return new objects (also for non-CoW this is the case) + # we always return new objects, regardless of CoW or not assert s2 is not s # and those trigger CoW when mutated From 055d14a21f29efc4904e81f50c78395413db2617 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Mon, 28 Nov 2022 13:11:41 +0100 Subject: [PATCH 3/4] add whatsnew note about API change --- doc/source/whatsnew/v2.0.0.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst index 97ee96d8be25d..6812f732d4c75 100644 --- a/doc/source/whatsnew/v2.0.0.rst +++ b/doc/source/whatsnew/v2.0.0.rst @@ -350,6 +350,10 @@ Other API changes - Changed behavior of :class:`Index` constructor with an object-dtype ``numpy.ndarray`` containing all-``bool`` values or all-complex values, this will now retain object dtype, consistent with the :class:`Series` behavior (:issue:`49594`) - Changed behavior of :meth:`DataFrame.shift` with ``axis=1``, an integer ``fill_value``, and homogeneous datetime-like dtype, this now fills new columns with integer dtypes instead of casting to datetimelike (:issue:`49842`) - :meth:`DataFrame.values`, :meth:`DataFrame.to_numpy`, :meth:`DataFrame.xs`, :meth:`DataFrame.reindex`, :meth:`DataFrame.fillna`, and :meth:`DataFrame.replace` no longer silently consolidate the underlying arrays; do ``df = df.copy()`` to ensure consolidation (:issue:`49356`) +- Creating a new DataFrame using a full slice on both axes with :attr:`~DataFrame.loc` + or :attr:`~DataFrame.iloc` (thus, ``df.loc[:, :]`` or ``df.iloc[:, :]``) now returns a + new DataFrame (shallow copy) instead of the original DataFrame, consistent with other + methods to get a full slice (for example ``df.loc[:]`` or ``df[:]``) (:issue:`49469`) - .. --------------------------------------------------------------------------- From 830fe0d809eea557a4473a7c1c6302baa7bb506e Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Fri, 9 Dec 2022 09:30:07 +0100 Subject: [PATCH 4/4] fixup test change --- pandas/tests/indexing/test_loc.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pandas/tests/indexing/test_loc.py b/pandas/tests/indexing/test_loc.py index 37c5122d3d244..6bd0806a42a5a 100644 --- a/pandas/tests/indexing/test_loc.py +++ b/pandas/tests/indexing/test_loc.py @@ -1097,6 +1097,7 @@ def test_identity_slice_returns_new_object( sliced_df = original_df.loc[:] assert sliced_df is not original_df assert original_df[:] is not original_df + assert original_df.loc[:, :] is not original_df # should be a shallow copy assert np.shares_memory(original_df["a"]._values, sliced_df["a"]._values) @@ -1110,10 +1111,6 @@ def test_identity_slice_returns_new_object( assert (sliced_df["a"] == 4).all() # These should not return copies - if using_copy_on_write: - assert original_df is not original_df.loc[:, :] - else: - assert original_df is original_df.loc[:, :] df = DataFrame(np.random.randn(10, 4)) assert df[0] is df.loc[:, 0]