BUG: Change in series copy behavior in 1.5

[anthonyaag]

Pandas version checks

• [X] I have checked that this issue has not already been reported.

• [X] I have confirmed this bug exists on the latest version of pandas.

• [X] I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd
ser1 = pd.Series([1,2,3,4])
ser2 = pd.Series(ser1)
ser3 = pd.Series(ser1, index=ser1.index)
print(ser1.index is ser2.index)
print(ser1.index is ser3.index)
print(ser1._mgr is ser2._mgr)
print(ser1._mgr is ser3._mgr)

Issue Description

Hi All!

Very excited to use pandas==1.5.0 but noticed this change. Not sure if its intentional as part of some of the series refactoring

In pandas 1.4.4 all of the above return true but in pandas 1.5.0 ser1._mgr is ser3._mgr returns False which is unexpected

Now when passed an index kwarg, that generates a new SingleBlockManager, but before 1.5 it didn't.

Expected Behavior

In [1]: import pandas as pd

In [2]: ser1 = pd.Series([1,2,3,4]) ...: ser2 = pd.Series(ser1) ...: ser3 = pd.Series(ser1, index=ser1.index) ...: print(ser1.index is ser2.index) ...: print(ser1.index is ser3.index) ...: print(ser1._mgr is ser2._mgr) ...: print(ser1._mgr is ser3._mgr) True True True True

Installed Versions

INSTALLED VERSIONS ------------------ commit : 87cfe4e38bafe7300a6003a1d18bd80f3f77c763 python : 3.10.7.final.0 python-bits : 64 OS : Linux OS-release : 5.4.117.hrtdev Version : #3 SMP Tue May 11 13:44:03 EDT 2021 machine : x86_64 processor : byteorder : little LC_ALL : LANG : en_US.UTF-8 LOCALE : en_US.UTF-8 pandas : 1.5.0 numpy : 1.23.2 pytz : 2022.1 dateutil : 2.8.2 setuptools : 62.1.0 pip : 22.2.2 Cython : 0.29.30 pytest : 7.1.2 hypothesis : 6.52.3 sphinx : 5.0.2 blosc : 1.10.6 feather : None xlsxwriter : None lxml.etree : 4.9.1 html5lib : 1.1 pymysql : 1.0.2 psycopg2 : 2.9.3 jinja2 : 3.1.2 IPython : 8.4.0 pandas_datareader: None bs4 : 4.11.1 bottleneck : 1.3.5 brotli : 1.0.9 fastparquet : 0.8.1 fsspec : 2022.5.0 gcsfs : None matplotlib : 3.5.2 numba : 0.56.2 numexpr : 2.8.3 odfpy : None openpyxl : 3.0.10 pandas_gbq : None pyarrow : 7.0.0 pyreadstat : None pyxlsb : 1.0.9 s3fs : None scipy : 1.9.1 snappy : None sqlalchemy : 1.4.39 tables : 3.7.0 tabulate : 0.8.10 xarray : 2022.6.0 xlrd : 2.0.1 xlwt : None zstandard : 0.18.0 tzdata : 2022.1

[phofl]

This was caused by

commit 221f6362bc25833da87f00015d4d5418ee316eff
Author: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Date:   Sat Aug 20 20:45:23 2022 +0200

    API: New copy / view semantics using Copy-on-Write (#46958)

cc @jorisvandenbossche was this intended?

In general, this is private. Does this leak somehow into the public api?

[phofl]

Adding milestone 1.5.1 for now

[jorisvandenbossche]

Thanks for the ping. I have to look into it in more depth, but from a quick look: this might be due to a change in reindex (when passing a Series object to the Series constructor, and also passing an index, then we reindex that series: https://github.com/pandas-dev/pandas/blob/73d15a7632e1b555defcc7942e5f629161626a4c/pandas/core/series.py#L438). Before, reindex() could return self, but in https://github.com/pandas-dev/pandas/pull/46958 I changed that to return a shallow copy (https://github.com/pandas-dev/pandas/pull/46958/files#diff-1a2e3df0db7dd8bddc2ec4bff9de8a7a55e328e6c32e2cecde761dc9549fcd46L5273-L5275).

That change in reindex itself was intentional (needed for CoW), but I didn't really consider the potential impact of it for reindex no longer returning self in case of an "identical" index (same values + same name). With 1.4, we have:

n [1]: s = pd.Series([1, 2, 3], index=['a', 'b', 'c'])

In [2]: s.reindex(s.index, copy=False) is s
Out[2]: True

In [3]: s.reindex(pd.Index(['a', 'b', 'c'], name="test"), copy=False) is s
Out[3]: False

In [4]: s.reindex(pd.Index(['a', 'b', 'c']), copy=False) is s
Out[4]: True

Personally, I would see this as a good change for reindex (to consistently return a new object, regardless of the index being equal or not). But it is a change in behaviour. So if we would like to preserve the old behaviour, it could easily be changed to only do this change if actually using CoW, and so the default behaviour wouldn't change.

---

@anthonyaag could you give a bit more detail how you ran into this specific issue? In what way to do rely on those managers being identical?

[anthonyaag]

With a few extension of a Series & Indexes. The test case that I found was breaking was basically testing almost exactly the above behavior.

        cs2 = CustomSeries(cs1)
        assert_series_equal(cs2, cs1)
        # this also changes cs1's index 
        cs2.index = CustomIndex(n=cs2.size)
        assert_series_equal(cs2, cs1)

So when the index stoped changing cs1's index then the test failed.

Also if this new behavior is better then that's fine too -- just wanted to raise it as it was a change. 😀

[jorisvandenbossche]

Interesting, that's a bit different from the code in the top post, though. Replicating this example with pure pandas (without subclass), I still see this behaviour of changing the second Series' index also updating the original one (using the latest main branch):

In [21]: s = pd.Series([1, 2, 3])

In [22]: s2 = pd.Series(s)

In [23]: s2.index = pd.Index(['a', 'b', 'c'])

In [24]: s.index
Out[24]: Index(['a', 'b', 'c'], dtype='object')

In [25]: pd.testing.assert_series_equal(s, s2)

But so the case in the top post is when passing an index to the constructor. And so also that case breaks here:

In [3]: s3 = pd.Series(s, index=s.index)

In [4]: s3.index = pd.Index([10, 11, 12])

In [5]: s.index
Out[5]: RangeIndex(start=0, stop=3, step=1)

In [6]: pd.testing.assert_series_equal(s, s3)
...
AssertionError: Series.index are different

In [7]: s3._mgr is s._mgr
Out[7]: False

Personally, seeing those examples, I very much prefer the behaviour where mutating one Series index propery doesn't change the other. But as mentioned above, it is clearly a breaking change, so we could also revert this for now, and only change it later (eg in 2.0, or 3.0).

_{(note: updated this after posting, I made an error while testing this, the last case did change behaviour, I initially wrote it didn't)}

[MarcoGorelli]

moving off the 2.0 milestone as it's a regression from 1.5