Opened 8 years ago

Last modified 5 years ago

#11791 needs_work enhancement

Introspection for interactively defined classes with metaclass

Reported by: SimonKing Owned by: jason
Priority: major Milestone: sage-6.4
Component: cython Keywords: introspection, cython, dynamic metaclass
Cc: Merged in:
Authors: Simon King Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11298, #11768, #11734, #11115, #10620, #9107 Stopgaps:

Description

#11298 provides the possibility to inspect interactive cython code. However, the following case did not work:

sage: cython_code = [
....: 'from sage.structure.dynamic_class import dynamic_class',
....: 'from sage.all import cached_function',
....: 'MyMetaclass = dynamic_class("MyMetaclass",(ClasscallMetaclass,))',
....: 'class Bar2:',
....: '    __metaclass__ = MyMetaclass',
....: '    @cached_function',
....: '    def __classcall__(cls, R):',
....: '        return type.__call__(cls, R)',
....: '    def __init__(self,R):',
....: '        self.R = R',
....: '    def __repr__(self):',
....: '        return "[%s]"%self.R',
....: '    def __hash__(self):',
....: '        print "computing the hash"',
....: '        return int(12345)']
sage: sage: cython('\n'.join(cython_code))
sage: Bar2??

It would show the code of ClasscallMetaclass, not the definition of Bar2.

The attached patch fixes it. It is based on cleaning-up sage_getsourcelines: That function uses different techniques to get the source lines, and it turns out that they were attempted in a wrong order.

The patch also needs #11768, or it wouldn't apply, and #11734, or sage would not start.

Attachments (1)

trac11791_dynamic_metaclass_introspection.patch (11.6 KB) - added by SimonKing 8 years ago.
Introspection for classes with dynamic metaclass

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by SimonKing

  • Status changed from new to needs_review

The attached patch seems to solve the problem. From the doctests:

        sage: cython_code = [
        ... 'from sage.structure.dynamic_class import dynamic_class',
        ... 'from sage.all import cached_function',
        ... 'MyMetaclass = dynamic_class("MyMetaclass",(ClasscallMetaclass,))',
        ... 'class Bar2:',
        ... '    __metaclass__ = MyMetaclass',
        ... '    @cached_function',
        ... '    def __classcall__(cls, R):',
        ... '        return type.__call__(cls, R)',
        ... '    def __init__(self,R):',
        ... '        self.R = R',
        ... '    def __repr__(self):',
        ... '        return "[%s]"%self.R',
        ... '    def __hash__(self):',
        ... '        print "computing the hash"',
        ... '        return int(12345)']
        sage: cython('\n'.join(cython_code))
        sage: print ''.join(sage_getsourcelines(Bar2)[0])
        class Bar2:
            __metaclass__ = MyMetaclass
            @cached_function
            def __classcall__(cls, R):
                return type.__call__(cls, R)
            def __init__(self,R):
                self.R = R
            def __repr__(self):
                return "[%s]"%self.R
            def __hash__(self):
                print "computing the hash"
                return int(12345)

The tests of sage.misc.sageinspect pass with the patch. I did not try full tests yet, but I think it could already be reviewed.

comment:2 Changed 8 years ago by SimonKing

For the record: The tests pass, and the documentation looks fine.

Now it's your turn, whoever is reading it!

comment:3 Changed 8 years ago by SimonKing

  • Dependencies changed from #11298, #11768, #11734 to #11298, #11768, #11734, #11115

Outch! I just realise that my patch adds a doctest that only works on top of #11115! Hence, adding it as a dependency!

comment:4 Changed 8 years ago by davidloeffler

  • Status changed from needs_review to needs_work
  • Work issues set to needs rebase

According to patchbot this fails to apply to 5.0.beta10 (there's a conflict in sageinspect.py). Your turn again, Simon, I'm afraid...

comment:5 Changed 8 years ago by SimonKing

  • Dependencies changed from #11298, #11768, #11734, #11115 to #11298, #11768, #11734, #11115, #10620
  • Status changed from needs_work to needs_review
  • Work issues needs rebase deleted

The conflict was with #10620 and is trivial: #10620 changed a pair of "s into a pair of 's -- and my old patch did the same.

It is updated and needs review (and so does #11768, by the way).

One question, though: I recall that somebody (Jeroen?) mentioned on sage-devel that one should mark all doc tests requiring a compiler (i.e., doctests using cython) as "optional - gcc". Is that true? Then the patch needs work.

Changed 8 years ago by SimonKing

Introspection for classes with dynamic metaclass

comment:6 Changed 8 years ago by SimonKing

  • Dependencies changed from #11298, #11768, #11734, #11115, #10620 to #11298, #11768, #11734, #11115, #10620, #9107

In some cases, sage_getfile would still not get the correct file name for interactively defined classes. Hence, the following would not work, even with #9107 applied:

age: cython_code = [
... "from sage.structure.unique_representation import UniqueRepresentation",
... "class A1(UniqueRepresentation):",
... "    class B1(UniqueRepresentation):",
... "        class C1: pass",
... "    class B2:",
... "        class C2: pass"]
sage: import os
sage: cython(os.linesep.join(cython_code))
sage: A1.B1.C1??          

I fixed the problem and turn it into a new doctest. However, that makes #9107 a new dependency (for making the test work).

I also used that occasion to use the new :trac: directive, and was a bit more careful by using os.extsep instead of '.' and so on.

comment:7 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

Needs a git branch

comment:12 Changed 5 years ago by jdemeyer

  • Component changed from misc to cython
Note: See TracTickets for help on using tickets.