Skip to content

Commit

Permalink
CoW warning mode: clean-up some TODOs (#56080)
Browse files Browse the repository at this point in the history
  • Loading branch information
jorisvandenbossche authored Nov 20, 2023
1 parent b193ac5 commit e7fa8bb
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 14 deletions.
3 changes: 2 additions & 1 deletion pandas/tests/copy_view/test_core_functionalities.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ def test_setitem_with_view_invalidated_does_not_copy(
df["b"] = 100
arr = get_array(df, "a")
view = None # noqa: F841
# TODO(CoW-warn) false positive?
# TODO(CoW-warn) false positive? -> block gets split because of `df["b"] = 100`
# which introduces additional refs, even when those of `view` go out of scopes
with tm.assert_cow_warning(warn_copy_on_write):
df.iloc[0, 0] = 100
if using_copy_on_write:
Expand Down
28 changes: 22 additions & 6 deletions pandas/tests/copy_view/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ def test_subset_column_slice(
subset.iloc[0, 0] = 0
assert not np.shares_memory(get_array(subset, "b"), get_array(df, "b"))
elif warn_copy_on_write:
# TODO(CoW-warn) should warn
with tm.assert_cow_warning(single_block):
subset.iloc[0, 0] = 0
else:
Expand Down Expand Up @@ -334,7 +333,6 @@ def test_subset_set_with_row_indexer(
):
pytest.skip("setitem with labels selects on columns")

# TODO(CoW-warn) should warn
if using_copy_on_write:
indexer_si(subset)[indexer] = 0
elif warn_copy_on_write:
Expand Down Expand Up @@ -369,7 +367,8 @@ def test_subset_set_with_mask(backend, using_copy_on_write, warn_copy_on_write):

mask = subset > 3

# TODO(CoW-warn) should warn
# TODO(CoW-warn) should warn -> mask is a DataFrame, which ends up going through
# DataFrame._where(..., inplace=True)
if using_copy_on_write or warn_copy_on_write:
subset[mask] = 0
else:
Expand Down Expand Up @@ -403,7 +402,6 @@ def test_subset_set_column(backend, using_copy_on_write, warn_copy_on_write):
else:
arr = pd.array([10, 11], dtype="Int64")

# TODO(CoW-warn) should warn
if using_copy_on_write or warn_copy_on_write:
subset["a"] = arr
else:
Expand Down Expand Up @@ -512,7 +510,6 @@ def test_subset_set_columns(backend, using_copy_on_write, warn_copy_on_write, dt
df_orig = df.copy()
subset = df[1:3]

# TODO(CoW-warn) should warn
if using_copy_on_write or warn_copy_on_write:
subset[["a", "c"]] = 0
else:
Expand Down Expand Up @@ -877,6 +874,8 @@ def test_series_subset_set_with_indexer(
)
if warn_copy_on_write:
# TODO(CoW-warn) should also warn for setting with mask
# -> Series.__setitem__ with boolean mask ends up using Series._set_values
# or Series._where depending on value being set
with tm.assert_cow_warning(
not is_mask, raise_on_extra_warnings=warn is not None
):
Expand Down Expand Up @@ -1006,6 +1005,7 @@ def test_column_as_series_set_with_upcast(
s[0] = "foo"
expected = Series([1, 2, 3], name="a")
elif using_copy_on_write or warn_copy_on_write or using_array_manager:
# TODO(CoW-warn) assert the FutureWarning for CoW is also raised
with tm.assert_produces_warning(FutureWarning, match="incompatible dtype"):
s[0] = "foo"
expected = Series(["foo", 2, 3], dtype=object, name="a")
Expand Down Expand Up @@ -1130,6 +1130,7 @@ def test_set_value_copy_only_necessary_column(
view = df[:]

if val == "a" and indexer[0] != slice(None):
# TODO(CoW-warn) assert the FutureWarning for CoW is also raised
with tm.assert_produces_warning(
FutureWarning, match="Setting an item of incompatible dtype is deprecated"
):
Expand All @@ -1154,6 +1155,8 @@ def test_series_midx_slice(using_copy_on_write):
ser = Series([1, 2, 3], index=pd.MultiIndex.from_arrays([[1, 1, 2], [3, 4, 5]]))
result = ser[1]
assert np.shares_memory(get_array(ser), get_array(result))
# TODO(CoW-warn) should warn -> reference is only tracked in CoW mode, so
# warning is not triggered
result.iloc[0] = 100
if using_copy_on_write:
expected = Series(
Expand All @@ -1162,7 +1165,9 @@ def test_series_midx_slice(using_copy_on_write):
tm.assert_series_equal(ser, expected)


def test_getitem_midx_slice(using_copy_on_write, using_array_manager):
def test_getitem_midx_slice(
using_copy_on_write, warn_copy_on_write, using_array_manager
):
df = DataFrame({("a", "x"): [1, 2], ("a", "y"): 1, ("b", "x"): 2})
df_orig = df.copy()
new_df = df[("a",)]
Expand All @@ -1175,6 +1180,15 @@ def test_getitem_midx_slice(using_copy_on_write, using_array_manager):
if using_copy_on_write:
new_df.iloc[0, 0] = 100
tm.assert_frame_equal(df_orig, df)
else:
if warn_copy_on_write:
with tm.assert_cow_warning():
new_df.iloc[0, 0] = 100
else:
with pd.option_context("chained_assignment", "warn"):
with tm.assert_produces_warning(SettingWithCopyWarning):
new_df.iloc[0, 0] = 100
assert df.iloc[0, 0] == 100


def test_series_midx_tuples_slice(using_copy_on_write):
Expand All @@ -1184,6 +1198,8 @@ def test_series_midx_tuples_slice(using_copy_on_write):
)
result = ser[(1, 2)]
assert np.shares_memory(get_array(ser), get_array(result))
# TODO(CoW-warn) should warn -> reference is only tracked in CoW mode, so
# warning is not triggered
result.iloc[0] = 100
if using_copy_on_write:
expected = Series(
Expand Down
3 changes: 0 additions & 3 deletions pandas/tests/copy_view/test_setitem.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import numpy as np
import pytest

from pandas import (
DataFrame,
Expand Down Expand Up @@ -67,8 +66,6 @@ def test_set_column_with_index(using_copy_on_write):
assert not np.shares_memory(get_array(df, "d"), arr)


# TODO(CoW-warn) this should NOT warn
@pytest.mark.filterwarnings("ignore:Setting a value on a view:FutureWarning")
def test_set_columns_with_dataframe(using_copy_on_write):
# Case: setting a DataFrame as new columns copies that data
# (with delayed copy with CoW)
Expand Down
2 changes: 0 additions & 2 deletions pandas/tests/frame/indexing/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1466,8 +1466,6 @@ def test_loc_named_tuple_for_midx(self):
)
tm.assert_frame_equal(result, expected)

# TODO(CoW-warn) shouldn't warn, but does because of item cache
@pytest.mark.filterwarnings("ignore:Setting a value on a view:FutureWarning")
@pytest.mark.parametrize("indexer", [["a"], "a"])
@pytest.mark.parametrize("col", [{}, {"b": 1}])
def test_set_2d_casting_date_to_int(self, col, indexer):
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/groupby/test_groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_repr():
assert result == expected


# TODO(CoW-warn) this should NOT warn
# TODO(CoW-warn) this should NOT warn -> inplace operator triggers it
@pytest.mark.filterwarnings("ignore:Setting a value on a view:FutureWarning")
def test_groupby_std_datetimelike(warn_copy_on_write):
# GH#48481
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/indexing/test_iloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ def test_iloc_getitem_slice_dups(self):
tm.assert_frame_equal(df.iloc[10:, :2], df2)
tm.assert_frame_equal(df.iloc[10:, 2:], df1)

# TODO(CoW-warn) this should NOT warn
# TODO(CoW-warn) this should NOT warn -> Series inplace operator
@pytest.mark.filterwarnings("ignore:Setting a value on a view:FutureWarning")
def test_iloc_setitem(self):
df = DataFrame(
Expand Down

0 comments on commit e7fa8bb

Please sign in to comment.