DEPR: enforce inplaceness for df.loc[:, foo]=bar #49775
Conversation
| @@ -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.
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.
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.
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 df.loc[:, 'a'] to df.loc[1, 'a'] (scalar), the it does raise a warning with a single block, but not for a df with multiple dtypes ...
jbrockmendel
added
the
Indexing
There was a problem hiding this comment.
Looks good! Added a question about the implementation (whether the internals already can do this as well), and a bunch of small nits on the tests.
If I do a search for "will attempt" in the codebase, there are a few remaining ones that can be cleaned-up:
- pandas/tests/frame/test_nonunique_indexes.py in
test_set_value_by_index - pandas/tests/frame/methods/test_sort_index.py a
mark.filterwarnings
| df.loc[:, "Alpha"] = categories | ||
| # pre-2.0 this swapped in a new array, in 2.0 it operates inplace, | ||
| # consistent with non-split-path | ||
| df.loc[:, "Alpha"] = categories |
There was a problem hiding this comment.
Do you think it would be worth to also test a non-inplace way (I suppose normal setitem?) to still somewhat cover the original bug?
(although I can imagine that those code paths are completely different, and different from the time of the bug, and maybe already tested elsewhere, so not necessarily worth it)
|
Everything here looks mostly address so going to merge. Can create follow up PRs if there are any followups needed |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.cc @jorisvandenbossche i expect the CoW-case edits in the copy_view tests are unwanted. pls advise.