Ticket #12357: trac12357_reviewer.patch

sage/categories/groupoid.pyx

@@ -35 +35 @@
     def __classcall__(cls, S):
         """
         In contrast to usual instances of
-        :class:`~sage.structure.unique_representation.UniqueRepresentation`,
-        the cache is implemented in the given argument.
-        
+        :class:`~sage.structure.unique_representation.UniqueRepresentation`
+        for which :meth:`__classcall__` is a
+        :class:`~sage.misc.cachefunc.cached_function`,
+        here the cache is implemented in the given argument.
+
         In that way, no "external" strong references to the groupoid are created
         by caching -- there only is a reference from ``Groupoid(G)`` to ``G``
-        and from ``G`` to ``Groupoid(G)``, but Python's garbage collection can
+        and from ``G`` to ``Groupoid(G)``, and Python's garbage collection can
         easily deal with those circular references.
 
         EXAMPLES::
@@ -73 +75 @@
             sage: Groupoid(1)
             Traceback (most recent call last):
             ...
-            TypeError: 1 must either allow attribute assignment or be instances of <type 'sage.structure.parent.Parent'>
+            TypeError: 1 must either allow attribute assignment or be an instance of <type 'sage.structure.parent.Parent'>
             sage: class A: pass
             sage: a = A()
             sage: Groupoid(a)
@@ -82 +84 @@
             True
 
         """
+        # First try the cache
         try:
             return getattr(S, '_cached_'+cls.__name__)
         except AttributeError:
             pass
-        cdef Parent G
-        cdef dict D
+
         if S is None:
             raise TypeError, "Groupoid of None is not defined"
 
+        # Make sure pickling works
+        # Copied from UniqueRepresentation.__classcall__
         instance = type.__call__(cls, S)
         assert(isinstance(instance,cls))
         if instance.__class__.__reduce__ == UniqueRepresentation.__reduce__:
             instance._reduction = (cls, (S,), {})
+
+        # Now try to store the Groupoid as an attribute
         try:
             setattr(S, '_cached_'+cls.__name__, instance)
             return instance
         except AttributeError:
             pass
-        if S is not None:
-            try:
-                # Parents have a cpdef dict __cached_methods attribute, by #11115
-                G = S
-                if G is None:
-                    raise ValueError
-                D = G.__cached_methods
-                if D is None:
-                    D = G.__cached_methods = {}
-                D['_cached_'+cls.__name__] = instance
-                return instance
-            except TypeError:
-                pass
-        raise TypeError, "%s must either allow attribute assignment or be instances of %s"%(S, Parent)
+
+        # Last try: S should be a Parent
+        cdef Parent G
+        cdef dict D
+        try:
+            # Parents have a cpdef dict __cached_methods attribute, by #11115
+            G = S
+            if G is None:
+                raise ValueError
+            D = G.__cached_methods
+            if D is None:
+                D = G.__cached_methods = {}
+            D['_cached_'+cls.__name__] = instance
+            return instance
+        except TypeError:
+            pass
+        raise TypeError, "%s must either allow attribute assignment or be an instance of %s"%(S, Parent)
 
     def __init__(self, G = None):
         """