CoW - don't try to update underlying values of Series/column inplace for inplace operator
State: closed

Related to

* https://github.com/pandas-dev/pandas/pull/37508
* https://github.com/pandas-dev/pandas/pull/51403

At the moment, an inplace operator, like `ser += 1` / `df["col"] += 1`, is actually first compute out of place, and then _if the dtypes match_, we update the original series' values inplace with the result's values. Essentially we do the following under the hood:

```python
# ser += 1
result = ser + 1
ser._values[:] = result._values
```

Except that when CoW is enabled, we first trigger a copy of the underlying values if needed (if the series has references). This happens here (`mgr.setitem_inplace()` will copy the block if needed):

https://github.com/pandas-dev/pandas/blob/984d75543fce5c68f0dcc4f0b3500256e4a9c0c9/pandas/core/generic.py#L12390-L12403

This current implementation has two downsides:

* The `setitem_inplace` can trigger a copy with CoW, and this copy is actually unnecessary since we can also simply replace the values instead of first copying and then updating this copy inplace (this makes `df["a"] += 1` currently _less_ efficient than `df["a"] = df["a"] + 1`, because the latter does not trigger an additional copy). 
* We sometimes update the values inplace, and sometimes not. Without CoW, at least we know exactly when (only for series and when the result has the exact same dtype), but with CoW it depends on whether the series has a reference or not. 
  The only way that this matters to the user is when they rely on having the underlying values and expect them to be updated (eg `ser = pd.Series(arr, copy=False); ser += 1` and expecting `arr` was also updated). We don't want user to rely on this (since with CoW it can be unreliable anyway), and so the more consistent behaviour would be to simply never update the original array inplace.

From a user perspective, it doesn't matter whether we update the underlying values inplace (`ser._values[:] = result._values`) or swap them out (like `ser._values = result._values`). And there is no efficiency reason to do it inplace, because we already calculated the result in a separate variable anyway, and we are replacing the all data anyway.

So I think we can simply decide that inplace operations on a full series or column only operate inplace on the pandas object, and will never update the underlying values. 

(note that this doesn't cover inplace operators with subsets, like `df.loc[0, 0] += 1` or `ser[0] += 1`)

xref https://github.com/pandas-dev/pandas/issues/48998

Labels:
name: Copy / view semantics

Comments:
> I don't like the inconsistency with subsets though, thoughts?

I think that if you see the inplace operator as kind of syntactic sugar:

```python
# df["col"] += 1
df["col"] = df["col"] + 1
# df.iloc[0, 0] += 1
df.iloc[0, 0] = df.iloc[0, 0] + 1
```

then it makes sense that the one is just replacing the column while the other one is inplace.

That analogy doesn't fully hold for `ser += 1`, though (although maybe `ser = ser + 1` comparison still makes sense, that would also replace your full values)
no strong opinion here.  seems like inplace arithmetic is something we want to discourage going forward since it'll have surprising-unless-you-carefully-read-the-docs behavior?
@jbrockmendel what is the "suprising" behaviour in your idea? The fact that it is not "inplace" in the sense like a numpy array does it inplace in the same memory place? 
Because that is something that has never been the case for pandas, this PR is not changing that (also before this PR, we were doing the actual computation "out of place", and then just writing the resulting values to the original data). And that is also something that you generally don't care about as a user (certainly with CoW, you should never rely on how the underlying values are treated, as they can always get copied when you do some mutation. But even before CoW there was no guarantee about it, eg when the dtype changed)

Also for a dataframe (`df["col"] += 1`) is still "inplace" if you consider it on the dataframe level: it does update the dataframe inplace.
thx @jorisvandenbossche