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)
Change History (13)
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
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
- 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
- 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
- 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.
comment:6 Changed 8 years ago by
- 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
- Milestone changed from sage-5.11 to sage-5.12
comment:8 Changed 6 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:9 Changed 6 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:10 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:12 Changed 5 years ago by
- Component changed from misc to cython
The attached patch seems to solve the problem. From the doctests:
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.