API / CoW: detect and raise error for chained assignment under Copy-on-Write
State: closed

xref https://github.com/pandas-dev/pandas/issues/48998

One of the consequences of the copy / view rules with the Copy-on-Write proposal is that direct chained assignement (i.e. `df[..][..] =  ..`, without using any intermediate variable like `sub = df[..]; sub[..] = ..`) consistently _never_ works (so not depending on which type of indexing operation (eg column selection vs row mask) or on the order of the operations). 

Given that this will be one of the significant backwards incompatible aspects of the CoW change (and so we will also need to add warnings for this in advance (before using / switching to CoW); but this PR is focusing on the eventual behaviour with CoW enabled, adding such warnings is for another PR), I think it can be useful to keep raising an error for this even after the CoW behaviour would have become the default (in addition to warning about it in advance). 
And given that this consistently _never_ works, I think it is also fine to keep raising an error in the future for this, as there should never be a reason (in the future) to actually do this.

This is somewhat similar to the `SettingWithCopyError` we already have (the error that can optionally be enabled, to get errors instead of warnings). But I decided to not reuse this error but create a new exception class, because it is different enough (it is raised (or not) in different situations; for example SettingWithCopyError will not raise for chained assignment in the cases it is know the work at the moment, and it can raise in non-chained cases (using an intermediate variable) while the new `ChainedAssignmentError` would solely focus on chained cases).

This includes the changes from https://github.com/pandas-dev/pandas/pull/49450 as well (the first commit here), since that change was needed to have the refcounting work correctly. But it was a sufficiently stand-alone change, so I broke it off in its own PR.

**How does this work?** I am relying on the refcount of the object on which `__setitem__` is being called. If you consider the two simple chained and non-chained cases:

```python
df[col][mask] = ..
# vs
subset = df[col]
subset[mask] = ..
```

In the first case, the temporary object (from `df[col]`) only lives in this chain, and doesn't have any references to it otherwise (and would also be cleaned up after the setitem operation), and does has a lower reference count compared to the second, non-chained case where the intermediate object (here called `subset`) is explicitly created by the user. In the second case we don't want an error (because this is valid code to update `subset`; because of triggering CoW it will just not update the parent `df`). 

From testing, the reference count in the first (chained) case seems to be 3, and if there is another reference to the object, it is always higher than 3, and for now this seems to be robustly so for our full test suite. 

One problem with this approach is that **it is CPython specific**. For example PyPy doesn't use refcounting, and so this PR won't work to raise the error on PyPy (I probably still need to add a platform check together with the refcount check, to avoid we raise an error about `sys.getrefcount` not being available on PyPy). For example the numpy `resize` method also uses refcounting for certain cases, and therefore requires the user to pass a keyword to disable this check on PyPy (https://github.com/numpy/numpy/pull/8050). 
I am not familiar enough with PyPy to know if there might be alternatives ways to check this when using PyPy. And it is certainly a downside of the current approach that it would not work consistently across Python implementations. But it is also mostly a user convenience to signal they are doing something that won't work, so my feeling is that in this case this difference can be OK (it is not that correct code would behave differently. The refcount is not used to know if CoW needs to happen or not (which affects actual output), but only to know if it would be useful to signal the user about code that has no effect anyway).




- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Copy / view semantics
name: Stale

Comments:
> One of the consequences of the copy / view rules with the Copy-on-Write proposal is that direct chained assignement (i.e. df[..][..] = .., without using any intermediate variable like sub = df[..]; sub[..] = ..) consistently never works (so not depending on which type of indexing operation (eg column selection vs row mask) or on the order of the operations)

this seems counter intuitive. isn't the this the entire rationale for copy-on-write ?
> > One of the consequences of the copy / view rules with the Copy-on-Write proposal is that direct chained assignement consistently never works ...
> 
> this seems counter intuitive. isn't the this the entire rationale for copy-on-write ?

@jreback No, one of the main goals of it is to have a clear and consistent rules, and in the proposal the main rule is *"any indexing operation returns a new object that behaves as a copy"*. 
If you apply this rule _consistently_, this has the consequence that it doesn't matter if you do `subset = df[..]; subset[..] = ..` or `df[..][..] = ..`, since in both cases the first `df[..]` returns a new object that behaves as a copy, and thus further modifying this resulting object doesn't modify the parent `df`. As a consequence, chained assignment consistently never works (as opposed to the current situation, where chained assignment _sometimes_ works depending on the exact indexing operation and the order of the steps in the chain). 

This is mentioned explicitly in the [proposal](https://docs.google.com/document/d/1ZCQ9mx3LBMy-nhwRl33_jgcvWo9IWdEfxDNQ2thyTb0/edit#heading=h.iexejdstiz8u) under discussion. Quoting the summary:

> Because every single indexing step behaves as a copy, this also means that with this proposal, “chained assignment” (with multiple setitem steps) will _never_ work.

And there is also an explicit section about it at [Chained assignment](https://docs.google.com/document/d/1ZCQ9mx3LBMy-nhwRl33_jgcvWo9IWdEfxDNQ2thyTb0/edit#heading=h.dbqr0xsneytk). It was also repeatedly called out as one of the major changes with this proposal (eg last bullet point in https://github.com/pandas-dev/pandas/issues/36195#issuecomment-702195996, one of the bullet points when I revived the discussion with the current proposal at https://github.com/pandas-dev/pandas/issues/36195#issuecomment-786654449, last bullet point of https://github.com/pandas-dev/pandas/issues/36195#issuecomment-940459824 of Kevin supporting this change, https://github.com/pandas-dev/pandas/issues/36195#issuecomment-1135014930, etc, as some references in case someone wants to reread the discussion on this topic), and mentioned on the pandas-dev mailing list thread about this.

Also note that this PR doesn't actually change any behaviour regarding that. The fact that chained assignment never works was already included in the initial PR https://github.com/pandas-dev/pandas/pull/46958 (I am only adding an error here to signal this behaviour better to the user; it could also be a warning). So if we want to further discuss the core aspect of chained assignment, let's use the original issue for this (https://github.com/pandas-dev/pandas/issues/36195)?


ok that fair - agree consistency here is the important point
Rebased this to get rid of the first commit now https://github.com/pandas-dev/pandas/pull/49450 is merged
> Do you want to keep the commented print statements?

No, that was just useful while implementing / debugging, and first wanted to make sure that all tests are passing ;) Will remove now since that seems to be the case.
I think pre-commit complains about missing match statements in the pytest raises calls
Refcounts work differently on PyPy (there is no `sys.getrefcount` doesn't exist there), so we won't be able tot give this informative error on PyPy (as mentioned in the top post). So I did put the check and error behind a `if not PyPy` check. 
I also already updated the tests to handle this, but this currently can't actually be tested in practice, since our PyPy test build is segfaulting (https://github.com/pandas-dev/pandas/issues/50817)
I don't think its too urgent to get this working on PyPy at the moment. When I added the PyPy build, in addition to the segfaults, I remember that there was a bunch of failures relating to hashing (I think our hash functions might have been completely broken on PyPy).

Once #50817 is fixed, I'll try to set up a mechanism to selectively enable some tests.
thx @jorisvandenbossche