-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
DEPR: enforce inplaceness for df.loc[:, foo]=bar #49775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
963be53
9a65009
bc515ea
d40230c
e8d1bb4
afe52eb
0917853
d5e0220
dbbf80c
d59137a
fca00fb
77e091c
f748ffe
34a6f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -340,7 +340,7 @@ def test_subset_set_column_with_loc(using_copy_on_write, using_array_manager, dt | |
# The (i)loc[:, col] inplace deprecation gets triggered here, ignore those | ||
# warnings and only assert the SettingWithCopyWarning | ||
with tm.assert_produces_warning( | ||
SettingWithCopyWarning, | ||
None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be considered as a false negative? (I would expect a warning here, but in general we are not super consistent about when we warn and when not) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont fully grok when we should be issuing the warning, but I think that no copies are being made here, so it would make sense to not get the warning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in the past, we have often raised warnings both when the actual object is a copy (and people might think it is a view) or a view (and people might think it is a copy), despite the exact name of the warning. Now, I don't know if we ever clearly defined in which cases the warning should be raised, though ;) It also seems to depend on the dtypes and the exact value that is being set, as I was running the above example on released pandas and eg when changing the setitem from |
||
raise_on_extra_warnings=not using_array_manager, | ||
): | ||
subset.loc[:, "a"] = np.array([10, 11], dtype="int64") | ||
|
@@ -351,7 +351,7 @@ def test_subset_set_column_with_loc(using_copy_on_write, using_array_manager, dt | |
index=range(1, 3), | ||
) | ||
tm.assert_frame_equal(subset, expected) | ||
if using_copy_on_write or using_array_manager: | ||
if using_copy_on_write: | ||
# original parent dataframe is not modified (CoW) | ||
tm.assert_frame_equal(df, df_orig) | ||
else: | ||
|
@@ -376,15 +376,15 @@ def test_subset_set_column_with_loc2(using_copy_on_write, using_array_manager): | |
# The (i)loc[:, col] inplace deprecation gets triggered here, ignore those | ||
# warnings and only assert the SettingWithCopyWarning | ||
with tm.assert_produces_warning( | ||
SettingWithCopyWarning, | ||
None, | ||
raise_on_extra_warnings=not using_array_manager, | ||
): | ||
subset.loc[:, "a"] = 0 | ||
|
||
subset._mgr._verify_integrity() | ||
expected = DataFrame({"a": [0, 0]}, index=range(1, 3)) | ||
tm.assert_frame_equal(subset, expected) | ||
if using_copy_on_write or using_array_manager: | ||
if using_copy_on_write: | ||
# original parent dataframe is not modified (CoW) | ||
tm.assert_frame_equal(df, df_orig) | ||
else: | ||
|
@@ -441,22 +441,20 @@ def test_subset_set_with_column_indexer( | |
with pd.option_context("chained_assignment", "warn"): | ||
# The (i)loc[:, col] inplace deprecation gets triggered here, ignore those | ||
# warnings and only assert the SettingWithCopyWarning | ||
jbrockmendel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
with tm.assert_produces_warning( | ||
SettingWithCopyWarning, raise_on_extra_warnings=False | ||
): | ||
subset.loc[:, indexer] = 0 | ||
# As of 2.0, this setitem attempts (successfully) to set values | ||
# inplace, so the assignment is not chained. | ||
subset.loc[:, indexer] = 0 | ||
|
||
subset._mgr._verify_integrity() | ||
expected = DataFrame({"a": [0, 0], "b": [0.0, 0.0], "c": [5, 6]}, index=range(1, 3)) | ||
# TODO full row slice .loc[:, idx] update inplace instead of overwrite? | ||
expected["b"] = expected["b"].astype("int64") | ||
tm.assert_frame_equal(subset, expected) | ||
if using_copy_on_write or using_array_manager: | ||
if using_copy_on_write: | ||
tm.assert_frame_equal(df, df_orig) | ||
else: | ||
# In the mixed case with BlockManager, only one of the two columns is | ||
# mutated in the parent frame .. | ||
df_orig.loc[1:2, ["a"]] = 0 | ||
# pre-2.0, in the mixed case with BlockManager, only column "a" | ||
# would be mutated in the parent frame. this changed with the | ||
# enforcement of GH#45333 | ||
df_orig.loc[1:2, ["a", "b"]] = 0 | ||
tm.assert_frame_equal(df, df_orig) | ||
|
||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.