Include an element's parent in _cache_key()

See http://trac.sagemath.org/ticket/16316#comment:8 for a discussion of this.

Author: saraedum

src/sage/misc/cachefunc.pyx

@@ -483,21 +483,21 @@ def _cache_key(o):
         sage: a = K(1); a
         1 + O(3^20)
         sage: _cache_key(a)
-        (((1,),), 0, 20)
+        (..., ((1,),), 0, 20)
 
     This function works if ``o`` is a tuple. In this case it unpacks its
     entries recursively::
 
         sage: o = (1, 2, (3, a))
         sage: _cache_key(o)
-        (1, 2, (3, (((1,),), 0, 20)))
+        (1, 2, (3, (..., ((1,),), 0, 20)))
 
     Note that tuples are only partially unpacked if some of its entries are
     hashable::
 
-        sage: o = (matrix([1, 2, 3]), a)
+        sage: o = (1/2, a)
         sage: _cache_key(o)
-        ([1 2 3], (((1,),), 0, 20))
+        (1/2, (..., ((1,),), 0, 20))
 
     .. SEEALSO::
 
@@ -844,7 +844,7 @@ cdef class CachedFunction(object):
             sage: f(y)
             1 + O(2^2)
             sage: f.cache
-            {(((((1,),), 0, 1),), ()): 1 + O(2), (((((1,),), 0, 2),), ()): 1 + O(2^2)}
+            {(((..., ((1,),), 0, 2),), ()): 1 + O(2^2), (((..., ((1,),), 0, 1),), ()): 1 + O(2)}
 
         """
         # We shortcut a common case of no arguments
@@ -1192,7 +1192,7 @@ cdef class WeakCachedFunction(CachedFunction):
             sage: f(y)
             (1 + O(2^20))*t + 1 + O(2^2)
             sage: list(f.cache.keys())
-            [((((((1,),), 0, 2), (((1,),), 0, 20)),), ()), ((((((1,),), 0, 1), (((1,),), 0, 20)),), ())]
+            [(((..., ((..., ((1,),), 0, 1), (..., ((1,),), 0, 20))),), ()), (((..., ((..., ((1,),), 0, 2), (..., ((1,),), 0, 20))),), ())]
 
         """
         # We shortcut a common case of no arguments
@@ -1744,7 +1744,7 @@ cdef class CachedMethodCaller(CachedFunction):
             sage: a.f(y)
             1 + O(2^2)
             sage: a.f.cache
-            {(((((1,),), 0, 1),), ()): 1 + O(2), (((((1,),), 0, 2),), ()): 1 + O(2^2)}
+            {(((..., ((1,),), 0, 2),), ()): 1 + O(2^2), (((..., ((1,),), 0, 1),), ()): 1 + O(2)}
 
         """
         if self._instance is None:

src/sage/rings/padics/padic_ZZ_pX_CR_element.pyx

@@ -474,8 +474,7 @@ cdef class pAdicZZpXCRElement(pAdicZZpXElement):
 
     def _cache_key(self):
         r"""
-        Return a hashable key which uniquely identifies this element among all
-        elements in the parent.
+        Return a hashable key which identifies this element.
 
         This enables caching for `p`-adic numbers which are not hashable.
 
@@ -502,26 +501,26 @@ cdef class pAdicZZpXCRElement(pAdicZZpXElement):
         ``_cache_key`` which is hashable and uniquely identifies them::
 
             sage: a._cache_key()
-            (((0, 1),), 0, 20)
+            (..., ((0, 1),), 0, 20)
             sage: b._cache_key()
-            (((0, 1),), 0, 1)
+            (..., ((0, 1),), 0, 1)
 
         TESTS:
 
         Check that zero values are handled correctly::
 
             sage: K.zero()._cache_key()
-            0
+            (..., 0)
             sage: K(0,1)._cache_key()
-            (0, 1)
+            (..., 0, 1)
 
         """
         if self._is_exact_zero():
-            return 0
+            return (id(self.parent()), 0)
         elif self._is_inexact_zero():
-            return 0, self.valuation()
+            return (id(self.parent()), 0, self.valuation())
         else:
-            return tuple(tuple(c) if isinstance(c,list) else c for c in self.unit_part().list()), self.valuation(), self.precision_relative()
+            return id(self.parent()), tuple(tuple(c) if isinstance(c,list) else c for c in self.unit_part().list()), self.valuation(), self.precision_relative()
 
     cdef int _set_inexact_zero(self, long absprec) except -1:
         """

src/sage/rings/polynomial/polynomial_element.pyx

@@ -14,7 +14,7 @@ AUTHORS:
 -  Simon King: Use a faster way of conversion from the base ring.
 
 -  Julian Rueth (2012-05-25,2014-05-09): Fixed is_squarefree() for imperfect
-   fields, fixed division without remainder over QQbar, added ``_cache_key``
+   fields, fixed division without remainder over QQbar; added ``_cache_key``
    for polynomials with unhashable coefficients
 
 -  Simon King (2013-10): Implement copying of :class:`PolynomialBaseringInjection`.
@@ -832,8 +832,7 @@ cdef class Polynomial(CommutativeAlgebraElement):
 
     def _cache_key(self):
         """
-        Return a key which uniquely identifies this element among all elements
-        in the parent.
+        Return a key which identifies this element.
 
         .. SEEALSO::
 
@@ -852,7 +851,7 @@ cdef class Polynomial(CommutativeAlgebraElement):
             ...
             TypeError: unhashable type: 'sage.rings.padics.padic_ZZ_pX_CR_element.pAdicZZpXCRElement'
             sage: f._cache_key()
-            (0, 1 + O(2^20))
+            (..., (0, 1 + O(2^20)))
 
             sage: @cached_function
             ....: def foo(t): return t
@@ -861,7 +860,7 @@ cdef class Polynomial(CommutativeAlgebraElement):
             (1 + O(2^20))*x
 
         """
-        return tuple(self)
+        return id(self.parent()), tuple(self)
 
     # you may have to replicate this boilerplate code in derived classes if you override
     # __richcmp__.  The python documentation at  http://docs.python.org/api/type-structs.html

src/sage/structure/sage_object.pyx

@@ -223,14 +223,13 @@ cdef class SageObject:
 
     def _cache_key(self):
         r"""
-        Return a key which uniquely identifies this objects for caching. The
-        output must be hashable itself or a tuple of objects which are hashable
-        or define a ``_cache_key``.
+        Return a key which identifies this objects for caching. The output must
+        be hashable itself or a tuple of objects which are hashable or define a
+        ``_cache_key``.
 
         This method will only be called if the object itself is not hashable.
 
-        For most objects this will just be the object itself. However, some
-        immutable objects (such as `p`-adic numbers) can not implement a
+        Some immutable objects (such as `p`-adic numbers) can not implement a
         reasonable hash function because their ``==`` operator has been
         modified to return ``True`` for objects which might behave differently
         in some computations::
@@ -266,33 +265,24 @@ cdef class SageObject:
             False
 
             sage: b._cache_key()
-            (((0, 1),), 0, 1)
+            (..., ((0, 1),), 0, 1)
             sage: c._cache_key()
-            (((0, 1), (1,)), 0, 20)
+            (..., ((0, 1), (1,)), 0, 20)
 
-        Note that such ``_cache_key`` often does not uniquely identify an
-        object in a strict sense::
+        An implementation must make sure that for elements ``a`` and ``b``, if ``a != b``, then also ``a._cache_key() != b._cache_key()``. In pratice this means that the ``_cache_key`` should always include the ``id`` of the parent.
 
             sage: S.<a> = Qq(4)
             sage: d = a + O(2)
-            sage: b._cache_key() == d._cache_key()
-            True
-
-        However, this kind of behaviour is common for many elements with
-        different parents. Special care has to be taken when mixing such
-        elements in caches::
-
-            sage: A = matrix([[1]],base_ring=GF(2))
-            sage: A.set_immutable()
-            sage: B = matrix([[1]],base_ring=GF(3))
-            sage: B.set_immutable()
-            sage: A == B
-            True
-            sage: hash(A) == hash(B)
-            True
+            sage: b._cache_key() == d._cache_key() # this would be True if the parents were not included
+            False
 
         """
-        return self
+        try:
+            hash(self)
+        except TypeError:
+            raise NotImplementedError("{} is not hashable and does not implement _cache_key()".format(type(self)))
+        else:
+            assert False, "_cache_key() must not be called for hashable elements"
 
     #############################################################################
     # DATABASE Related code