fix cache_readonly

Author: fjetter

pandas/_libs/properties.pyx

@@ -24,11 +24,28 @@ cdef class CachedProperty:
 
         # Get the cache or set a default one if needed
         cache = getattr(obj, '_cache', None)
+
+        if cache is not None:
+            # When accessing cython extension types, the attribute is already
+            # registered and known to the class, unlike for python object. To
+            # ensure we're not accidentally using a global scope / class level
+            # cache we'll need to check whether the instance and class
+            # attribute is identical
+            cache_class = getattr(typ, "_cache", None)
+            if cache_class is not None and cache_class is cache:
+                raise TypeError(
+                    f"Class {typ} defines a `_cache` attribute on class level "
+                    "which is forbidden in combination with @cache_readonly."
+                )
+
         if cache is None:
             try:
                 cache = obj._cache = {}
             except (AttributeError):
-                return self
+                raise TypeError(
+                    f"Cython extension type {type(obj)} must declare attribute "
+                    "`_cache` to use @cache_readonly."
+                )
 
         if PyDict_Contains(cache, self.name):
             # not necessary to Py_INCREF

pandas/core/dtypes/dtypes.py

@@ -21,6 +21,7 @@
 import pytz
 
 from pandas._libs.interval import Interval
+from pandas._libs.properties import cache_readonly
 from pandas._libs.tslibs import (
     BaseOffset,
     NaT,
@@ -86,7 +87,7 @@ class PandasExtensionDtype(ExtensionDtype):
     base: Optional[DtypeObj] = None
     isbuiltin = 0
     isnative = 0
-    _cache: Dict[str_type, PandasExtensionDtype] = {}
+    _cache_dtypes: Dict[str_type, PandasExtensionDtype] = {}
 
     def __str__(self) -> str_type:
         """
@@ -110,7 +111,7 @@ def __getstate__(self) -> Dict[str_type, Any]:
     @classmethod
     def reset_cache(cls) -> None:
         """ clear the cache """
-        cls._cache = {}
+        cls._cache_dtypes = {}
 
 
 class CategoricalDtypeType(type):
@@ -182,7 +183,7 @@ class CategoricalDtype(PandasExtensionDtype, ExtensionDtype):
     str = "|O08"
     base = np.dtype("O")
     _metadata = ("categories", "ordered")
-    _cache: Dict[str_type, PandasExtensionDtype] = {}
+    _cache_dtypes: Dict[str_type, PandasExtensionDtype] = {}
 
     def __init__(self, categories=None, ordered: Ordered = False):
         self._finalize(categories, ordered, fastpath=False)
@@ -351,17 +352,6 @@ def __setstate__(self, state: MutableMapping[str_type, Any]) -> None:
         self._categories = state.pop("categories", None)
         self._ordered = state.pop("ordered", False)
 
-    def __hash__(self) -> int:
-        # _hash_categories returns a uint64, so use the negative
-        # space for when we have unknown categories to avoid a conflict
-        if self.categories is None:
-            if self.ordered:
-                return -1
-            else:
-                return -2
-        # We *do* want to include the real self.ordered here
-        return int(self._hash_categories(self.categories, self.ordered))
-
     def __eq__(self, other: Any) -> bool:
         """
         Rules for CDT equality:
@@ -434,14 +424,28 @@ def __repr__(self) -> str_type:
             data = data.rstrip(", ")
         return f"CategoricalDtype(categories={data}, ordered={self.ordered})"
 
-    @staticmethod
-    def _hash_categories(categories, ordered: Ordered = True) -> int:
+    def __hash__(self) -> int:
+        # _hash_categories returns a uint64, so use the negative
+        # space for when we have unknown categories to avoid a conflict
+        if self.categories is None:
+            if self.ordered:
+                return -1
+            else:
+                return -2
+        return int(self._hash_categories)
+
+    @cache_readonly
+    def _hash_categories(self) -> int:
         from pandas.core.util.hashing import (
             combine_hash_arrays,
             hash_array,
             hash_tuples,
         )
 
+        # We *do* want to include the real self.ordered here
+        categories = self.categories
+        ordered = self.ordered
+
         if len(categories) and isinstance(categories[0], tuple):
             # assumes if any individual category is a tuple, then all our. ATM
             # I don't really want to support just some of the categories being
@@ -678,7 +682,7 @@ class DatetimeTZDtype(PandasExtensionDtype):
     na_value = NaT
     _metadata = ("unit", "tz")
     _match = re.compile(r"(datetime64|M8)\[(?P<unit>.+), (?P<tz>.+)\]")
-    _cache: Dict[str_type, PandasExtensionDtype] = {}
+    _cache_dtypes: Dict[str_type, PandasExtensionDtype] = {}
 
     def __init__(self, unit: Union[str_type, DatetimeTZDtype] = "ns", tz=None):
         if isinstance(unit, DatetimeTZDtype):
@@ -844,7 +848,7 @@ class PeriodDtype(dtypes.PeriodDtypeBase, PandasExtensionDtype):
     num = 102
     _metadata = ("freq",)
     _match = re.compile(r"(P|p)eriod\[(?P<freq>.+)\]")
-    _cache: Dict[str_type, PandasExtensionDtype] = {}
+    _cache_dtypes: Dict[str_type, PandasExtensionDtype] = {}
 
     def __new__(cls, freq=None):
         """
@@ -866,12 +870,12 @@ def __new__(cls, freq=None):
             freq = cls._parse_dtype_strict(freq)
 
         try:
-            return cls._cache[freq.freqstr]
+            return cls._cache_dtypes[freq.freqstr]
         except KeyError:
             dtype_code = freq._period_dtype_code
             u = dtypes.PeriodDtypeBase.__new__(cls, dtype_code)
             u._freq = freq
-            cls._cache[freq.freqstr] = u
+            cls._cache_dtypes[freq.freqstr] = u
             return u
 
     def __reduce__(self):
@@ -1049,7 +1053,7 @@ class IntervalDtype(PandasExtensionDtype):
     _match = re.compile(
         r"(I|i)nterval\[(?P<subtype>[^,]+)(, (?P<closed>(right|left|both|neither)))?\]"
     )
-    _cache: Dict[str_type, PandasExtensionDtype] = {}
+    _cache_dtypes: Dict[str_type, PandasExtensionDtype] = {}
 
     def __new__(cls, subtype=None, closed: Optional[str_type] = None):
         from pandas.core.dtypes.common import (
@@ -1106,12 +1110,12 @@ def __new__(cls, subtype=None, closed: Optional[str_type] = None):
 
         key = str(subtype) + str(closed)
         try:
-            return cls._cache[key]
+            return cls._cache_dtypes[key]
         except KeyError:
             u = object.__new__(cls)
             u._subtype = subtype
             u._closed = closed
-            cls._cache[key] = u
+            cls._cache_dtypes[key] = u
             return u
 
     @property

pandas/tests/dtypes/test_dtypes.py

@@ -66,15 +66,15 @@ def test_pickle(self, dtype):
 
         # clear the cache
         type(dtype).reset_cache()
-        assert not len(dtype._cache)
+        assert not len(dtype._cache_dtypes)
 
         # force back to the cache
         result = tm.round_trip_pickle(dtype)
         if not isinstance(dtype, PeriodDtype):
             # Because PeriodDtype has a cython class as a base class,
             #  it has different pickle semantics, and its cache is re-populated
             #  on un-pickling.
-            assert not len(dtype._cache)
+            assert not len(dtype._cache_dtypes)
         assert result == dtype
 
 
@@ -791,14 +791,14 @@ def test_basic_dtype(self):
     def test_caching(self):
         IntervalDtype.reset_cache()
         dtype = IntervalDtype("int64", "right")
-        assert len(IntervalDtype._cache) == 1
+        assert len(IntervalDtype._cache_dtypes) == 1
 
         IntervalDtype("interval")
-        assert len(IntervalDtype._cache) == 2
+        assert len(IntervalDtype._cache_dtypes) == 2
 
         IntervalDtype.reset_cache()
         tm.round_trip_pickle(dtype)
-        assert len(IntervalDtype._cache) == 0
+        assert len(IntervalDtype._cache_dtypes) == 0
 
     def test_not_string(self):
         # GH30568: though IntervalDtype has object kind, it cannot be string