Bug Ensure cache_readonly is only applied on instance
State: closed

The property cache was using the attribute `_cache` which occasionally is used by other classes as a class attribute. Most prominently this is true for all subclasses of `PandasExtensionDtype`, see https://github.com/pandas-dev/pandas/blob/ce082fb04a0cf597f673d35bc47afd6c2e3c3168/pandas/core/dtypes/dtypes.py#L89

This would cause the same cash to be used for the property caching as for the extension dtype. The dtype cache is a class attribute whereas the property cache is supposed to be instance only. I fixed this by using a longer name for the property caching attribute, hoping this would lead to less collisions.

Introducing a new attribute leads to failures for Cython extension types which previously were handled with an `AttributeError`. Firstly, I believe these cases should not silently pass and secondly the previous behaviour was wrong by not executing the function w/out cache but it returned the instance of the `CachedProperty` class instead. I addressed this but enabling the caching for extension classes requires defining the attribute explicitly. I went ahead and created a mixin class for this but I'm open to other suggestions as well.


- [x] closes N/A
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Bug
name: Performance
description: Memory or execution speed performance
name: Categorical
description: Categorical Data Type

Comments:
Wouldn't it be simpler to just not instantiate _cache on the class?
> Wouldn't it be simpler to just not instantiate _cache on the class?

We don't need a cache per instance not per class.
I believe renaming the property cache is not the best idea since this is used all over the place. The actual problem is that these extension type caches https://github.com/pandas-dev/pandas/blob/7b5957f8dc0f608c328b363efcb467671ce23a35/pandas/core/dtypes/dtypes.py#L89

are class attributes but share the same name as the commonly used instance attribute and I do not know how best to distinguish them. I would propose to rename the dtypes cache but I'm not sure if this is perceived as a breaking change or not
>  I would propose to rename the dtypes cache but I'm not sure if this is perceived as a breaking change or not

It would not.  Go for it.
Why not go the other direction and change PandasExtensionDtype._cache to e.g. PandasExtensionDtype._instance_cache
Sorry for not responding. I was out of office for a while.

I added a runtime check to the property cache implementation which is supposed to ensure that we're not accessing any class wide state. I'm also ensuring that we're actually using the cache for ExtensionTypes and raise an appropriate exception if we do not. This led me down the serialization implementation of the `Offset` classes where most implement a custom serialization function although there is one reduce implemented which should serve all and ensures that the classes are properly instantiated. Otherwise we run into this runtime check I implemented.

@jbrockmendel 
I get that the complexity in `properties.pyx` is not necessary. I wanted to have some kind of protection that this is not happening again and this runtime check is the only way I could come up with. Code runs either way with the changes in `dtype` but I could not come up with another way of testing this behaviour. If this is fine without a dedicated test, I can revert the cython code
I reverted the changes in the cython code base. What's left is the renaming of the `ExtensionDtype._cache`
@fjetter looks fine, can you merge master and ping on green
This was closed by #40193