Refactor getattr_from_other_class() for lookup of methods in categories

[nthiery]

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.

Depends on #21030
Depends on #21409

Upstream: Fixed upstream, in a later stable release.

CC: @jdemeyer @tscrim

Component: categories

Author: Jeroen Demeyer

Branch/Commit: 74041f3

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/20686

[jdemeyer]

comment:2

I might have some ideas... discuss in Meudon?

[nthiery]

comment:3

+1

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Branch: u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes

[jdemeyer]

Dependencies: #20825

[jdemeyer]

New commits:

a143a32	EvaluationMethods should be a new-style class
bbd0b05	Improve getattr_from_other_class

[jdemeyer]

Commit: bbd0b05

[sagetrac-git]

Changed commit from bbd0b05 to 3cd449f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

44e80b3	EvaluationMethods should be a new-style class
3cd449f	Improve getattr_from_other_class

[jdemeyer]

Upstream: Reported upstream. No feedback yet.

[jdemeyer]

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e906b7e

Improve getattr_from_other_class

[sagetrac-git]

Changed commit from 3cd449f to e906b7e

[sagetrac-git]

Changed commit from e906b7e to 92f4520

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

92f4520

Improve getattr_from_other_class

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9f529b0

Improve getattr_from_other_class

[sagetrac-git]

Changed commit from 92f4520 to 9f529b0

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ff3a496

Improve getattr_from_other_class

[sagetrac-git]

Changed commit from 9f529b0 to ff3a496

[videlec]

comment:72

And also not related to _an_element_

**********************************************************************
File "src/sage/combinat/root_system/integrable_representations.py", line 189, in sage.combinat.root_system.integrable_representations.IntegrableRepresentation.
__init__
Failed example:
    TestSuite(V).run()
Expected nothing
Got:
    ...
    Failure in _test_not_implemented_methods:
    Traceback (most recent call last):
    ...
    AssertionError: Not implemented method: __contains__
    ...

[jdemeyer]

comment:73

The last patchbot report was green, so this must be a recently-introduced problem.

[jdemeyer]

comment:74

Interestingly, these failures are for classes inheriting from CategoryObject but not Parent.

[jdemeyer]

comment:75

Those failures are true bugs in HochschildComplex and IntegrableRepresentation: they pretend to be in some category but do not actually implement everything needed to be in that category. This ticket causes the category tests to be run (which is good) but then the tests fail.

sage: SGA = SymmetricGroupAlgebra(QQ, 3)
sage: T = SGA.trivial_representation()
sage: H = SGA.hochschild_complex(T)
sage: H in CommutativeAdditiveGroups()
True
sage: H.zero()
...
TypeError: 'HochschildComplex' object is not callable

and

sage: Lambda = RootSystem(['A',3,1]).weight_lattice(extended=true).fundamental_weights()
sage: V = IntegrableRepresentation(Lambda[1]+Lambda[2]+Lambda[3])
sage: V in CommutativeAdditiveGroups()
True
sage: V.zero()
...
TypeError: 'IntegrableRepresentation' object is not callable

I think the right thing to do here is to mark those tests as # known bug.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

74041f3

Mark broken testsuites as # known bug

[sagetrac-git]

Changed commit from 0fabb7a to 74041f3

[jdemeyer]

comment:77

I created #21386 and #21387 for the broken testsuites.

[tscrim]

comment:78

I hope to fix both today.

[videlec]

comment:79

good to go!

[vbraun]

Changed dependencies from #21030 to #21030, #21409

[vbraun]

comment:80

This ticket seems to cause #21409

[vbraun]

Changed branch from u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes to 74041f3