Trac #20686: Refactor getattr_from_other_class() for lookup of method…

…s in categories

For parents and elements implemented in Cython, category lookup is
emulated in `__getattr__` using `getattr_from_other_class`. In this
ticket, this is refactored a bit:

1. We should not pick up special attributes from `type`, there is no
point in returning the type name of the category here (similarly for
`__dict__`, `__mro__`, ...):
{{{
sage: ZZ(1).__name__
'JoinCategory.element_class'
sage: ZZ.__name__
'JoinCategory.parent_class'
}}}
This change causes a few failures which need to be fixed.

2. The implementation of `getattr_from_other_class` is not very robust.
For example, static methods are not supported. We fix this by using low-
level Python functions to get the attribute and we manually call the
descriptor `__get__` if needed.

3. We shouldn't do anything special with double-underscore private
attributes: in plain Python, this is implemented by the parser and not
by `getattr()`. So `__getattr__` would already receive the mangled
private name.

4. Some of the changes broke `_sage_src_lines_`, so we also change that:
currently, `_sage_src_lines_()` is used both to get the source lines of
a class (e.g. dynamic classes) and an instance (e.g. cached functions).
We change this to always mean the source lines of an instance, which
makes things clearer.

5. The lookup using `getattr_from_other_class` is about 9 times slower
than a normal method lookup::
{{{
sage: def foo(self): return
sage: Sets().element_class.foo = foo
sage: def g(x):
....:     for i in range(1000):
....:          x.foo()

sage: x = Semigroups().example().an_element()
sage: y = 1

sage: %timeit g(x)
10000 loops, best of 3: 115 µs per loop

sage: %timeit g(y)
1000 loops, best of 3: 1.18 ms per loop
}}}
We improve on this by roughly a factor 2 (but even then, it's still a
lot slower than usual method lookup).

NOTE: This needs a trivial Cython patch, see #21030 for the Cython
upgrade.

URL: https://trac.sagemath.org/20686
Reported by: nthiery
Ticket author(s): Jeroen Demeyer
Reviewer(s): Vincent Delecroix

src/doc/en/thematic_tutorials/tutorial-objects-and-classes.rst

@@ -297,17 +297,17 @@ Some particular actions modify the data structure of ``el``::
         sage: type(e)
         <type 'sage.rings.integer.Integer'>
         sage: e.__dict__
-        <dictproxy {'__doc__': ...
-         '_sage_src_lines_': <staticmethod object at 0x...>}>
-        sage: e.__dict__.keys()
-        ['__module__', '_reduction', '__doc__', '_sage_src_lines_']
+        Traceback (most recent call last):
+        ...
+        AttributeError: 'sage.rings.integer.Integer' object has no attribute '__dict__'
 
         sage: id4 = SymmetricGroup(4).one()
         sage: type(id4)
         <type 'sage.groups.perm_gps.permgroup_element.SymmetricGroupElement'>
         sage: id4.__dict__
-        <dictproxy {'__doc__': ...
-        '_sage_src_lines_': <staticmethod object at 0x...>}>
+        Traceback (most recent call last):
+        ...
+        AttributeError: 'sage.groups.perm_gps.permgroup_element.SymmetricGroupElement' object has no attribute '__dict__'
 
 .. note::
 

src/sage/categories/map.pyx

@@ -154,8 +154,7 @@ cdef class Map(Element):
 
             sage: phi = QQ['x']._internal_coerce_map_from(ZZ)
             sage: phi.domain
-            <weakref at ...; to 'sage.rings.integer_ring.IntegerRing_class'
-            at ... (JoinCategory.parent_class)>
+            <weakref at ...; to 'sage.rings.integer_ring.IntegerRing_class' at ...>
             sage: type(phi)
             <type 'sage.categories.map.FormalCompositeMap'>
             sage: psi = copy(phi)   # indirect doctest

src/sage/combinat/root_system/integrable_representations.py

@@ -186,7 +186,10 @@ def __init__(self, Lam):
 
             sage: Lambda = RootSystem(['A',3,1]).weight_lattice(extended=true).fundamental_weights()
             sage: V = IntegrableRepresentation(Lambda[1]+Lambda[2]+Lambda[3])
-            sage: TestSuite(V).run()
+
+        Some methods required by the category are not implemented::
+
+            sage: TestSuite(V).run()  # known bug (#21387)
         """
         CategoryObject.__init__(self, base=ZZ, category=Modules(ZZ))
 

src/sage/functions/hypergeometric.py

@@ -998,7 +998,7 @@ def _derivative_(self, a, b, z, diff_param):
         raise NotImplementedError('derivative of hypergeometric function '
                                   'with respect to parameters')
 
-    class EvaluationMethods:
+    class EvaluationMethods(object):
         def generalized(cls, self, a, b, z):
             """
             Return as a generalized hypergeometric function
@@ -1098,7 +1098,7 @@ def _derivative_(self, a, b, z, diff_param):
         raise NotImplementedError('derivative of hypergeometric function '
                                   'with respect to parameters')
 
-    class EvaluationMethods:
+    class EvaluationMethods(object):
         def generalized(cls, self, a, b, z):
             """
             Return in terms of the generalized hypergeometric function

src/sage/geometry/toric_plotter.py

@@ -261,7 +261,7 @@ def __eq__(self, other):
             False
         """
         # Just to make TestSuite happy...
-        return self.__dict__ == other.__dict__
+        return type(self) is type(other) and self.__dict__ == other.__dict__
 
     def adjust_options(self):
         r"""

src/sage/homology/hochschild_complex.py

@@ -92,14 +92,14 @@ def __init__(self, A, M):
             sage: T = SGA.trivial_representation()
             sage: H = SGA.hochschild_complex(T)
 
-        We skip the category test because the category containment
-        tests assumes ``H`` is an instance of :class:`Parent`::
-
-            sage: TestSuite(H).run(skip="_test_category")
-            sage: H.category() == ChainComplexes(QQ)
-            True
-            sage: H in ChainComplexes(QQ) # known bug
+            sage: H.category()
+            Category of chain complexes over Rational Field
+            sage: H in ChainComplexes(QQ)
             True
+
+        Some methods required by the category are not implemented::
+
+            sage: TestSuite(H).run()  # known bug (#21386)
         """
         self._A = A
         self._M = M

src/sage/misc/sageinspect.py

@@ -1283,7 +1283,7 @@ def sage_getargspec(obj):
 
         sage: from sage.misc.sageinspect import sage_getargspec
         sage: def f(x, y, z=1, t=2, *args, **keywords):
-        ...       pass
+        ....:     pass
         sage: sage_getargspec(f)
         ArgSpec(args=['x', 'y', 'z', 't'], varargs='args', keywords='keywords', defaults=(1, 2))
 
@@ -1294,13 +1294,12 @@ def sage_getargspec(obj):
         sage: sage_getargspec(factor)
         ArgSpec(args=['n', 'proof', 'int_', 'algorithm', 'verbose'], varargs=None, keywords='kwds', defaults=(None, False, 'pari', 0))
 
-
     In the case of a class or a class instance, the ``ArgSpec`` of the
     ``__new__``, ``__init__`` or ``__call__`` method is returned::
 
         sage: P.<x,y> = QQ[]
         sage: sage_getargspec(P)
-        ArgSpec(args=['self', 'x'], varargs='args', keywords='kwds', defaults=(0,))
+        ArgSpec(args=['base_ring', 'n', 'names', 'order'], varargs=None, keywords=None, defaults=('degrevlex',))
         sage: sage_getargspec(P.__class__)
         ArgSpec(args=['self', 'x'], varargs='args', keywords='kwds', defaults=(0,))
 
@@ -1324,8 +1323,8 @@ def sage_getargspec(obj):
     If a ``functools.partial`` instance is involved, we see no other meaningful solution
     than to return the argspec of the underlying function::
 
-        sage: def f(a,b,c,d=1): return a+b+c+d
-        ...
+        sage: def f(a,b,c,d=1):
+        ....:     return a+b+c+d
         sage: import functools
         sage: f1 = functools.partial(f, 1,c=2)
         sage: sage_getargspec(f1)
@@ -1436,7 +1435,10 @@ def foo(x, a='\')"', b={not (2+1==3):'bar'}): return
         pass
     # If we are lucky, the function signature is embedded in the docstring.
     docstring = _sage_getdoc_unformatted(obj)
-    name = obj.__name__ if hasattr(obj,'__name__') else type(obj).__name__
+    try:
+        name = obj.__name__
+    except AttributeError:
+        name = type(obj).__name__
     argspec = _extract_embedded_signature(docstring, name)[1]
     if argspec is not None:
         return argspec
@@ -1984,6 +1986,16 @@ def sage_getsourcelines(obj):
         sage: sage_getsourcelines(matrix)[0][0][6:]
         'MatrixFactory(object):\n'
 
+    Some classes customize this using a ``_sage_src_lines_`` method,
+    which gives the source lines of a class instance, but not the class
+    itself. We demonstrate this for :class:`CachedFunction`::
+
+        sage: cachedfib = cached_function(fibonacci)
+        sage: sage_getsourcelines(cachedfib)[0][0]
+        'def fibonacci(n, algorithm="pari"):\n'
+        sage: sage_getsourcelines(type(cachedfib))[0][0]
+         'cdef class CachedFunction(object):\n'
+
     TESTS::
 
         sage: cython('''cpdef test_funct(x,y): return''')
@@ -2051,7 +2063,7 @@ class Element:
         sage: sage_getsourcelines(HC)
         (['    class Homsets(HomsetsCategory):\n', ...], ...)
 
-    Testing against a bug that has occured during work on #11768::
+    Testing against a bug that has occured during work on :trac:`11768`::
 
         sage: P.<x,y> = QQ[]
         sage: I = P*[x,y]
@@ -2071,18 +2083,27 @@ class Element:
     - Simon King: If a class has no docstring then let the class
       definition be found starting from the ``__init__`` method.
     - Simon King: Get source lines for dynamic classes.
-
     """
-
+    # First try the method _sage_src_lines_(), which is meant to give
+    # the source lines of an object (not of its type!).
     try:
-        return obj._sage_src_lines_()
+        sage_src_lines = obj._sage_src_lines_
     except AttributeError:
         pass
-    except TypeError:
-        # That happes for instances of dynamic classes
-        return sage_getsourcelines(obj.__class__)
+    else:
+        try:
+            return sage_src_lines()
+        except (NotImplementedError, TypeError):
+            # NotImplementedError can be raised by _sage_src_lines_()
+            # to indicate that it didn't find the source lines.
+            #
+            # TypeError can happen when obj is a type and
+            # obj._sage_src_lines_ is an unbound method. In this case,
+            # we don't want to use _sage_src_lines_(), we just want to
+            # get the source of the type itself.
+            pass
 
-    # Check if we deal with instance
+    # Check if we deal with an instance
     if isclassinstance(obj):
         if isinstance(obj,functools.partial):
             return sage_getsourcelines(obj.func)

src/sage/structure/category_object.pxd

@@ -14,6 +14,7 @@ from sage.structure.generators cimport Generators
 cpdef inline check_default_category(default_category, category)
 
 cdef class CategoryObject(SageObject):
+    cdef public dict __cached_methods
     cdef _generators
     cdef _category
     cdef public _base
@@ -22,5 +23,7 @@ cdef class CategoryObject(SageObject):
     cdef object __weakref__
     cdef long _hash_value
 
+    cdef getattr_from_category(self, name)
+
 cpdef normalize_names(Py_ssize_t ngens, names)
 cpdef bint certify_names(names) except -1

src/sage/structure/category_object.pyx

@@ -46,19 +46,19 @@ This example illustrates generators for a free module over `\ZZ`.
     ((1, 0, 0, 0), (0, 1, 0, 0), (0, 0, 1, 0), (0, 0, 0, 1))
 """
 
-from __future__ import division
-
 #*****************************************************************************
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation, either version 2 of the License, or
 # (at your option) any later version.
 #                  http://www.gnu.org/licenses/
 #*****************************************************************************
-from __future__ import print_function
 
-cimport generators
-cimport sage_object
+from __future__ import absolute_import, division, print_function
+
+cimport sage.structure.generators as generators
+from sage.structure.misc import dir_with_other_class
+from sage.structure.misc cimport getattr_from_other_class
 from sage.categories.category import Category
 from sage.structure.debug_options import debug
 from sage.misc.cachefunc import cached_method
@@ -102,7 +102,7 @@ cpdef inline check_default_category(default_category, category):
         return default_category
     return default_category.join([default_category,category])
 
-cdef class CategoryObject(sage_object.SageObject):
+cdef class CategoryObject(SageObject):
     """
     An object in some category.
     """
@@ -140,11 +140,12 @@ cdef class CategoryObject(sage_object.SageObject):
         """
         if base is not None:
             self._base = base
-        self._generators = {}
         if category is not None:
             self._init_category_(category)
 
     def __cinit__(self):
+        self.__cached_methods = {}
+        self._generators = {}
         self._hash_value = -1
 
     def _init_category_(self, category):
@@ -819,6 +820,131 @@ cdef class CategoryObject(sage_object.SageObject):
             self._hash_value = hash(repr(self))
         return self._hash_value
 
+    ##############################################################################
+    # Getting attributes from the category
+    ##############################################################################
+
+    def __getattr__(self, name):
+        """
+        Let cat be the category of ``self``. This method emulates
+        ``self`` being an instance of both ``CategoryObject`` and
+        ``cat.parent_class``, in that order, for attribute lookup.
+
+        This attribute lookup is cached for speed.
+
+        EXAMPLES:
+
+        We test that ZZ (an extension type) inherits the methods from
+        its categories, that is from ``EuclideanDomains().parent_class``::
+
+            sage: ZZ._test_associativity
+            <bound method JoinCategory.parent_class._test_associativity of Integer Ring>
+            sage: ZZ._test_associativity(verbose = True)
+            sage: TestSuite(ZZ).run(verbose = True)
+            running ._test_additive_associativity() . . . pass
+            running ._test_an_element() . . . pass
+            running ._test_associativity() . . . pass
+            running ._test_cardinality() . . . pass
+            running ._test_category() . . . pass
+            running ._test_characteristic() . . . pass
+            running ._test_distributivity() . . . pass
+            running ._test_elements() . . .
+              Running the test suite of self.an_element()
+              running ._test_category() . . . pass
+              running ._test_eq() . . . pass
+              running ._test_nonzero_equal() . . . pass
+              running ._test_not_implemented_methods() . . . pass
+              running ._test_pickling() . . . pass
+              pass
+            running ._test_elements_eq_reflexive() . . . pass
+            running ._test_elements_eq_symmetric() . . . pass
+            running ._test_elements_eq_transitive() . . . pass
+            running ._test_elements_neq() . . . pass
+            running ._test_enumerated_set_contains() . . . pass
+            running ._test_enumerated_set_iter_cardinality() . . . pass
+            running ._test_enumerated_set_iter_list() . . .Enumerated set too big; skipping test; increase tester._max_runs
+             pass
+            running ._test_eq() . . . pass
+            running ._test_euclidean_degree() . . . pass
+            running ._test_gcd_vs_xgcd() . . . pass
+            running ._test_metric() . . . pass
+            running ._test_not_implemented_methods() . . . pass
+            running ._test_one() . . . pass
+            running ._test_pickling() . . . pass
+            running ._test_prod() . . . pass
+            running ._test_quo_rem() . . . pass
+            running ._test_some_elements() . . . pass
+            running ._test_zero() . . . pass
+            running ._test_zero_divisors() . . . pass
+
+            sage: Sets().example().sadfasdf
+            Traceback (most recent call last):
+            ...
+            AttributeError: 'PrimeNumbers_with_category' object has no attribute 'sadfasdf'
+        """
+        return self.getattr_from_category(name)
+
+    cdef getattr_from_category(self, name):
+        # Lookup a method or attribute from the category abstract classes.
+        # See __getattr__ above for documentation.
+        try:
+            return self.__cached_methods[name]
+        except KeyError:
+            if self._category is None:
+                # Usually, this will just raise AttributeError in
+                # getattr_from_other_class().
+                cls = type
+            else:
+                cls = self._category.parent_class
+
+            attr = getattr_from_other_class(self, cls, name)
+            self.__cached_methods[name] = attr
+            return attr
+
+    def __dir__(self):
+        """
+        Let cat be the category of ``self``. This method emulates
+        ``self`` being an instance of both ``CategoryObject`` and
+        ``cat.parent_class``, in that order, for attribute directory.
+
+        EXAMPLES::
+
+            sage: for s in dir(ZZ):
+            ....:     if s[:6] == "_test_": print(s)
+            _test_additive_associativity
+            _test_an_element
+            _test_associativity
+            _test_cardinality
+            _test_category
+            _test_characteristic
+            _test_distributivity
+            _test_elements
+            _test_elements_eq_reflexive
+            _test_elements_eq_symmetric
+            _test_elements_eq_transitive
+            _test_elements_neq
+            _test_enumerated_set_contains
+            _test_enumerated_set_iter_cardinality
+            _test_enumerated_set_iter_list
+            _test_eq
+            _test_euclidean_degree
+            _test_gcd_vs_xgcd
+            _test_metric
+            _test_not_implemented_methods
+            _test_one
+            _test_pickling
+            _test_prod
+            _test_quo_rem
+            _test_some_elements
+            _test_zero
+            _test_zero_divisors
+            sage: F = GF(9,'a')
+            sage: dir(F)
+            [..., '__class__', ..., '_test_pickling', ..., 'extension', ...]
+
+        """
+        return dir_with_other_class(self, self.category().parent_class)
+
     ##############################################################################
     # For compatibility with Python 2
     ##############################################################################