BUG: caching in CachedAccessor is problematic and (probably) unnecessary

[kdebrab]

Pandas version checks

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

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

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

Reproducible Example

import pandas as pd
ser = pd.Series(["a", "b", "c"])

print(ser.str.strip().dtype)  # object: OK
print(ser.astype("string").str.strip().dtype)  # string: OK
ser.astype("string", copy=False)
print(ser.str.strip().dtype)  # object: not OK

Issue Description

Caching pandas Series/Frame accessors like plot, str, etc.. seems to be problematic. See also pydata/xarray#3268 (pandas accessors were ported from xarray)

Caching creates some unwanted side effects:

1. In case of the str accessor, it can lead to bugs (see example above)
2. Circular reference, well explained in Stateful user-defined accessors pydata/xarray#3268 (replace DataArray/Dataset by DataFrame/Series):

This design also carries the problem that it introduces a circular reference in the DataArray/Dataset. This means that, after someone invokes an accessor method on his DataArray/Dataset, then the whole object - including the numpy buffers! - won't be instantly collected when it's dereferenced by the user, and it will have to instead wait for the next gc pass. This could cause huge increases in RAM usage overnight in a user application, which would be very hard to logically link to a change that just added a custom method.

And illustrated for pandas by:

import gc
import weakref
import pandas as pd

ser = pd.Series(["a", "b", "c"])
w = weakref.ref(ser)
print("No accessor, in scope:", w() is not None)
del ser
print("No accessor, descoped:", w() is not None)

ser = pd.Series(["a", "b", "c"])
ser.str.strip()
w = weakref.ref(ser)
print("No accessor, in scope:", w() is not None)
del ser
print("No accessor, descoped:", w() is not None)
gc.collect()
print("with accessor, after gc pass:", w() is not None)

Output:

No accessor, in scope: True
No accessor, descoped: False
with accessor, in scope: True
with accessor, descoped: True  # Not OK
with accessor, after gc pass: False

Expected Behavior

The circular reference could be solved by using a weakref as explained in pydata/xarray#3268 (comment).

The bugs with the str accessor could be resolved by not initializing any attribute but _data in the StringMethods class.

However, I think the best solution is getting rid of the caching logic (which could be achieved by removing a single line:

pandas/pandas/core/accessor.py

Line 187 in 01b86ed

object.__setattr__(obj, self._name, accessor_obj)

, but one could also bypass the accessor class and simply declare the accessor object as a property of the owner). The performance loss of not caching seems marginal and in any case "less worse" than the circular reference issue.

Installed Versions

INSTALLED VERSIONS

commit : e8093ba
python : 3.10.2.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19043
machine : AMD64
processor : Intel64 Family 6 Model 78 Stepping 3, GenuineIntel
byteorder : little
LC_ALL : None
LANG : None
LOCALE : English_United Kingdom.1252
pandas : 1.4.3
numpy : 1.23.0
pytz : 2022.1
dateutil : 2.8.2
setuptools : 58.1.0
pip : 22.1.2
Cython : None
pytest : 7.0.1
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : 3.0.3
lxml.etree : 4.9.1
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.1.1
IPython : 8.1.0
pandas_datareader: None
bs4 : 4.10.0
bottleneck : 1.3.5
brotli : None
fastparquet : None
fsspec : 2022.02.0
gcsfs : None
markupsafe : 2.1.0
matplotlib : 3.5.2
numba : None
numexpr : 2.8.3
odfpy : None
openpyxl : 3.0.10
pandas_gbq : None
pyarrow : 8.0.0
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : 1.8.1
snappy : None
sqlalchemy : 1.4.32
tables : None
tabulate : 0.8.9
xarray : 2022.3.0
xlrd : None
xlwt : None
zstandard : None

[mzeitlin11]

Thanks for reporting this @kdebrab! #41357 looks related, possibly some other memory leak issues as well under https://github.com/pandas-dev/pandas/issues?q=is%3Aopen+is%3Aissue+label%3APerformance+memory. I think this would be a welcome change if no material slowdown was shown. CachedAccessor probably also causes typing-related complexities and contributes to subtle bugs like #38979

[lithomas1]

Hm. This might be slightly trickier than it looks.

I'm not familiar with this section of this code, but it looks like this could cause a perf degradation for Accessor classes that do stuff like validating data in the __init__ method.

I'll think about this more.

BTW, I don't think example is a bug. ser is still an object type Series after the astype.

[lithomas1]

Forgot to post this earlier but maybe storing the created accessor objects in a WeakKeyDictionary and having the DataFrame/Series as keys?

EDIT: This won't work.