Maintain inplace operation on series
State: closed

- [ ] closes #36373
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Copy / view semantics

Comments:
Hello @samilAyoub! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:









There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-09-25 19:30:33 UTC
It looks like the underlying problem is

```
ser = pd.Series([1, 2, 3])
vals = ser._values
ser += 1
assert ser._values is vals   # <-- Fails!
```
> It looks like the underlying problem is

but that also occurs in 1.0.5 (and on 0.25.3)
```
>>> pd.__version__
'1.0.5'
>>>
>>> ser = pd.Series([1, 2, 3])
>>> vals = ser._values
>>>
>>> ser += 1
>>> assert ser._values is vals  # <-- Fails!
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError
```
so maybe not directly related to regression? 
@jbrockmendel I think ready if you can have a look. Thanks.
>> It looks like the underlying problem is

> but that also occurs in 1.0.5 (and on 0.25.3)

So fixing this would fix two bugs instead of one.  Have you taken a look at what it would take to fix this?  I suspect it may be simpler than the implementation here.
@simonjayhawkins this may need some more work, don't block on for 1.1.3
> > > It looks like the underlying problem is
> 
> > but that also occurs in 1.0.5 (and on 0.25.3)
> 
> So fixing this would fix two bugs instead of one. Have you taken a look at what it would take to fix this?

It appears to me that the issue is that an inplace operation on the Series deletes the weakref to the dataframe, i.e. the fix in #30501 was incorrect

> I suspect it may be simpler than the implementation here.

This is simple. revert the incorrect fix. remove the test that has since been added for a issue that was not actually fixed. and finally ensure that dropped columns are not added back to original dataframe (i.e. what imo should have been done in #30501 originally).

Including fixes for the deleted test, i.e. issue #15631 would only make the change more invasive?

I'm not sure I understand how a more invasive change would be a backport candidate.
@simonjayhawkins your appraisal seems reasonable for backporting purposes.  i'll take a look a the `ser.values` issue and see how that affects options for longer-term solutions.
@samilAyoub 

can we just revert the original. (and pls merge master as well)
@simonjayhawkins this PR is stale, but I think if we revert the original commit is best here.
> @simonjayhawkins this PR is stale, but I think if we revert the original commit is best here.

if we do that, I think we would still need to disable or remove test_loc_copy_vs_view see https://github.com/pandas-dev/pandas/issues/15631#issuecomment-699074229

> > @simonjayhawkins this PR is stale, but I think if we revert the original commit is best here.
> 
> if we do that, I think we would still need to disable or remove test_loc_copy_vs_view see [#15631 (comment)](https://github.com/pandas-dev/pandas/issues/15631#issuecomment-699074229)

right that's what i mean revert that patch
I opened a PR with a revert of the original patch: https://github.com/pandas-dev/pandas/pull/37497
> I opened a PR with a revert of the original patch: #37497

test_loc_copy_vs_view will fail.


Ah, yes, thanks for the note, added an xfail 
(I am personally not really sure the test is actually correct, though, but that's another discussion ;-))
This PR can probably be claused now with https://github.com/pandas-dev/pandas/pull/37508 merged?

@samilAyoub sorry about sidetracking this PR, but wanted to go fast yesterday to be able to get a fix into 1.1.4 release today
Thanks @samilAyoub for working on this.