Sage: Ticket #11768: Get source code for parent/element classes of categories
https://trac.sagemath.org/ticket/11768
<p>
Currently (sage-4.7.2.alpha2) one can not interactively get the source code of the parent or element classes of a category:
</p>
<pre class="wiki">sage: C.parent_class??
Type: DynamicMetaclass
Base Class: <class 'sage.structure.dynamic_class.DynamicMetaclass'>
String Form: <class 'sage.categories.rings.Rings.parent_class'>
Namespace: Interactive
File: /mnt/local/king/SAGE/sage-4.7.2.alpha1/local/lib/python2.6/site-packages/sage/categories/rings.py
Source:
class DynamicMetaclass(type):
"""
A metaclass implementing an appropriate reduce-by-construction method
"""
def __reduce__(self):
"""
See sage.structure.dynamic_class.dynamic_class? for non trivial tests.
TESTS::
...
</pre><p>
As one can see, it gets the source file right, but can not find the class definition in it.
</p>
<p>
I tried to track it down.
</p>
<ul><li>For a dynamic metaclass, we want to obtain the documentation and source of the base class (resp. the class that is passed too the dynamic metaclass by the <code>doccls</code> argument).
</li><li>The module is correctly provided, and thus Python's inspect module is able to find the source file.
</li><li>If it is a class then Python's inspect module tries to find something like <code>"class "+cls.__name__+":"</code> in the sources (they use a more sophisticated regular expression).
</li><li>The base class is <code>Rings().ParentMethods</code>. Unfortunately, it is defined by <code>class ParentMethods</code>, but the name is differently:
<pre class="wiki">sage: Rings().ParentMethods.__name__
'Rings.ParentMethods'
</pre></li></ul><p>
I do not suggest to change the name: If the name is just <code>'ParentMethods'</code> then Python's inspect module would always find the first occurence of that name, which is bad if one module defines several categories.
</p>
<p>
Instead, I suggest to create a new function in <code>sage.misc.sageinspect</code> that recursively finds the sources for classes whose sources can't be found by other methods and whose name contains a dot:
</p>
<p>
The function that I plan to write does not start with the whole source file of <code>sage.categories.rings</code>, but it starts with the sources of <code>sage.categories.rings.Rings</code> and then searches <code>class ParentMethods</code> (with the regular expression from the inspect module) only in that small part of the full source file.
</p>
<p>
I think "getting sources" belongs to the component "misc", even though the problem is relevant for not getting confused by the category framework. Thus, cc to Nicolas...
</p>
<p>
<strong><span class="underline">Apply</span></strong>
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11768/trac11768_source_of_dynamic_class.patch" title="Attachment 'trac11768_source_of_dynamic_class.patch' in Ticket #11768">trac11768_source_of_dynamic_class.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11768/trac11768_source_of_dynamic_class.patch" title="Download"></a>
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11768
Trac 1.1.6SimonKingThu, 01 Sep 2011 10:08:56 GMT
https://trac.sagemath.org/ticket/11768#comment:1
https://trac.sagemath.org/ticket/11768#comment:1
<p>
The patch works exactly as it is announced in the ticket description. One can now (on top of sage-4.7.2.alpha2) do:
</p>
<pre class="wiki">sage: C = Rings()
sage: from sage.misc.sageinspect import sage_getsource
sage: print sage_getsource(C.parent_class) #indirect doctest
class ParentMethods:
def is_field(self):
raise NotImplementedError
<BLANKLINE>
def bracket(self, x, y):
...
</pre><p>
Unfortunately,
</p>
<pre class="wiki">sage: P = JackPolynomialsP(QQ); P.rename("JackP")
sage: P.element_class??
</pre><p>
does not work yet. So, the search for definitions should also include the <code>__base__</code> attribute of a class.
</p>
TicketSimonKingThu, 01 Sep 2011 10:52:47 GMTstatus changed; author set
https://trac.sagemath.org/ticket/11768#comment:2
https://trac.sagemath.org/ticket/11768#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>author</strong>
set to <em>Simon King</em>
</li>
</ul>
<p>
It seems to me that things are fine now.
</p>
<p>
In particular, we have
</p>
<pre class="wiki">sage: C = Rings()
sage: C.parent_class??
Type: DynamicMetaclass
Base Class: <class 'sage.structure.dynamic_class.DynamicMetaclass'>
String Form: <class 'sage.categories.rings.Rings.parent_class'>
Namespace: Interactive
File: /mnt/local/king/SAGE/sage-4.7.2.alpha1/local/lib/python2.6/site-packages/sage/categories/rings.py
Source:
class ParentMethods:
def is_field(self):
raise NotImplementedError
def bracket(self, x, y):
...
sage: P = JackPolynomialsP(QQ)
sage: P.element_class??
Type: DynamicMetaclass
Base Class: <class 'sage.structure.dynamic_class.DynamicMetaclass'>
String Form: <class 'sage.combinat.sf.jack.JackPolynomials_p_with_category.element_class'>
Namespace: Interactive
File: /mnt/local/king/SAGE/sage-4.7.2.alpha1/local/lib/python2.6/site-packages/sage/combinat/sf/jack.py
Definition: P.element_class(self, x, include=None, exclude=None)
Source:
class SymmetricFunctionAlgebra_generic_Element(CombinatorialFreeModule.Element):
def __repr__(self):
"""
EXAMPLES::
sage: m = SFAMonomial(QQ)
...
sage: P.<x,y> = QQ[]
sage: I = P*[x,y]
sage: I??
Type: MPolynomialIdeal
Base Class: <class 'sage.rings.polynomial.multi_polynomial_ideal.MPolynomialIdeal'>
String Form: Ideal (x, y) of Multivariate Polynomial Ring in x, y over Rational Field
Namespace: Interactive
File: /mnt/local/king/SAGE/sage-4.7.2.alpha1/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py
Source:
class MPolynomialIdeal( MPolynomialIdeal_singular_repr, \
MPolynomialIdeal_macaulay2_repr, \
MPolynomialIdeal_magma_repr, \
Ideal_generic ):
def __init__(self, ring, gens, coerce=True):
r"""
Create an ideal in a multivariate polynomial ring.
...
</pre><p>
All three examples are doctested (via <code>sage_getsourcelines</code>). So, ready for review!
</p>
TicketSimonKingThu, 01 Sep 2011 10:53:32 GMTmilestone changed
https://trac.sagemath.org/ticket/11768#comment:3
https://trac.sagemath.org/ticket/11768#comment:3
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.0</em> to <em>sage-4.7.2</em>
</li>
</ul>
<p>
Perhaps it would work to get it in sage-4.7.2, not only sage-5.0?
</p>
TicketSimonKingThu, 01 Sep 2011 11:42:13 GMTstatus changed
https://trac.sagemath.org/ticket/11768#comment:4
https://trac.sagemath.org/ticket/11768#comment:4
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
I just found one thing that doesn't work, yet.
</p>
<p>
With
</p>
<pre class="wiki">sage: C = Rings()
sage: HC = C.hom_category()
</pre><p>
neither <code>HC??</code> nor <code>edit(HC,'vim')</code> do what one would wish.
</p>
TicketSimonKingThu, 01 Sep 2011 12:05:32 GMTstatus changed
https://trac.sagemath.org/ticket/11768#comment:5
https://trac.sagemath.org/ticket/11768#comment:5
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Introspection of <code>Rings().hom_category()</code> works and is doctested with the updated patch. Back to "needs review".
</p>
TicketnthieryThu, 01 Sep 2011 19:55:40 GMT
https://trac.sagemath.org/ticket/11768#comment:6
https://trac.sagemath.org/ticket/11768#comment:6
<p>
Hi Simon!
</p>
<p>
Definitely +1 on the feature! Thanks!
</p>
<p>
I had a quick look at the implementation, and it sounds reasonable, though I do not yet have an opinion yet. Any sageinspect expert around?
</p>
TickethivertFri, 02 Sep 2011 11:45:09 GMTcc changed
https://trac.sagemath.org/ticket/11768#comment:7
https://trac.sagemath.org/ticket/11768#comment:7
<ul>
<li><strong>cc</strong>
<em>hivert</em> added
</li>
</ul>
<p>
I'm also +1 on the feature. I just want point out the following trap. With double class nesting the name mangling is currently incorrect (see <a class="closed ticket" href="https://trac.sagemath.org/ticket/9107" title="defect: Nested class name mangling can be wrong in case of double nesting (closed: fixed)">#9107</a>). Unfortunately, I won't have time to work on it (ping Nicolas ;-). There is a patch in combinat's queue ( trac_9107-nested_class_bug-fh.patch) which experiment on what is happening.
</p>
TicketSimonKingFri, 02 Sep 2011 15:12:45 GMT
https://trac.sagemath.org/ticket/11768#comment:8
https://trac.sagemath.org/ticket/11768#comment:8
<p>
Hi Florent,
</p>
<p>
Thank you for pointing to the other ticket! <a class="closed ticket" href="https://trac.sagemath.org/ticket/9107" title="defect: Nested class name mangling can be wrong in case of double nesting (closed: fixed)">#9107</a> seems quite mysterious to me. Clearly, in the example exposed there, my way to get to the sources would fail. Of course, the current way would fail even more...
</p>
TicketvbraunMon, 19 Sep 2011 09:37:30 GMT
https://trac.sagemath.org/ticket/11768#comment:9
https://trac.sagemath.org/ticket/11768#comment:9
<p>
Fails to apply to vanilla sage-4.7.2.alpha2. Dependencies?
</p>
TicketSimonKingMon, 19 Sep 2011 09:46:15 GMT
https://trac.sagemath.org/ticket/11768#comment:10
https://trac.sagemath.org/ticket/11768#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:9" title="Comment 9">vbraun</a>:
</p>
<blockquote class="citation">
<p>
Fails to apply to vanilla sage-4.7.2.alpha2. Dependencies?
</p>
</blockquote>
<p>
Actually I don't know (I don't remember that I had anything else added when I worked on the patch). But it should apply to sage-4.7.2.alpha3-prerelease.
</p>
TicketSimonKingMon, 19 Sep 2011 09:55:16 GMT
https://trac.sagemath.org/ticket/11768#comment:11
https://trac.sagemath.org/ticket/11768#comment:11
<p>
Do you really start with vanilla sage-4.7.2.alpha2? I just tested, and I have (sorry for the German):
</p>
<pre class="wiki">king@mpc622:/mnt/local/king/SAGE/debug/sage-4.7.2.alpha2/devel/sage$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11768/trac11768_source_of_dynamic_class.patch
Füge trac11768_source_of_dynamic_class.patch zur Seriendatei hinzu
king@mpc622:/mnt/local/king/SAGE/debug/sage-4.7.2.alpha2/devel/sage$ hg qpush
Wende trac11768_source_of_dynamic_class.patch an
jetzt bei: trac11768_source_of_dynamic_class.patch
king@mpc622:/mnt/local/king/SAGE/debug/sage-4.7.2.alpha2/devel/sage$ hg qapplied
trac11768_source_of_dynamic_class.patch
</pre><p>
So, it <em>does</em> apply to vanilla sage-4.7.2.alpha2 without fuzz.
</p>
TicketvbraunMon, 19 Sep 2011 10:47:12 GMT
https://trac.sagemath.org/ticket/11768#comment:12
https://trac.sagemath.org/ticket/11768#comment:12
<p>
Sorry, you are right. It does apply. Must have forgotten to pop all patches %-)
</p>
TicketSimonKingFri, 30 Sep 2011 08:02:20 GMT
https://trac.sagemath.org/ticket/11768#comment:13
https://trac.sagemath.org/ticket/11768#comment:13
<p>
What is the patchbot complaining about? It states that it fails to apply, but I find it is fine with 4.7.2-alpha3-prerelease.
</p>
TicketSimonKingWed, 26 Oct 2011 19:08:35 GMT
https://trac.sagemath.org/ticket/11768#comment:14
https://trac.sagemath.org/ticket/11768#comment:14
<p>
Any reviewer for that ticket? I think introspection is a nice thing to have in more generality...
</p>
TicketSimonKingSat, 29 Oct 2011 06:35:17 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/11768#comment:15
https://trac.sagemath.org/ticket/11768#comment:15
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>Make it independent of #11900</em>
</li>
</ul>
<p>
It turns out that a combination of this ticket with <a class="closed ticket" href="https://trac.sagemath.org/ticket/11900" title="defect: Serious regression caused by #9138 (closed: fixed)">#11900</a> yields a problem. I'll try to rewrite the patch here, so that it doesn't matter whether <a class="closed ticket" href="https://trac.sagemath.org/ticket/11900" title="defect: Serious regression caused by #9138 (closed: fixed)">#11900</a> is applied or not.
</p>
TicketSimonKingSat, 29 Oct 2011 07:25:03 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/11768#comment:16
https://trac.sagemath.org/ticket/11768#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>Make it independent of #11900</em> deleted
</li>
</ul>
<p>
Done! Using the new patch, the doc tests of sage/misc/sageinspect.py pass regardless whether <a class="closed ticket" href="https://trac.sagemath.org/ticket/11900" title="defect: Serious regression caused by #9138 (closed: fixed)">#11900</a> is applied or not.
</p>
TicketSimonKingThu, 10 Nov 2011 19:19:27 GMT
https://trac.sagemath.org/ticket/11768#comment:17
https://trac.sagemath.org/ticket/11768#comment:17
<p>
With the patch, one still has
</p>
<pre class="wiki">sage: C = Algebras(ZZ).TensorProducts()
sage: C.ParentMethods??
Error getting source: could not get source code
Type: classobj
String Form: sage.categories.algebras.TensorProducts.ParentMethods
Namespace: Interactive
File: /mnt/local/king/SAGE/rebase/sage-4.8.alpha0/local/lib/python2.6/site-packages/sage/categories/algebras.py
</pre><p>
However, if C is the category of Z-algebras, then <code>C.ParentMethods??</code> works nicely. So, the patch isn't perfect, but it is a progress. Review, anyone?
</p>
TicketSimonKingWed, 02 May 2012 19:24:15 GMT
https://trac.sagemath.org/ticket/11768#comment:18
https://trac.sagemath.org/ticket/11768#comment:18
<p>
If <a class="closed ticket" href="https://trac.sagemath.org/ticket/12876" title="enhancement: Fix element and parent classes of Hom categories to be abstract, and ... (closed: fixed)">#12876</a> is applied then one of the tests introduced here gives an unexpected result: We test against a particular code snipped, that changes with <a class="closed ticket" href="https://trac.sagemath.org/ticket/12876" title="enhancement: Fix element and parent classes of Hom categories to be abstract, and ... (closed: fixed)">#12876</a>.
</p>
<p>
Hence, I'll rewrite the test so that it expects only those parts of the code that do not change.
</p>
TicketSimonKingThu, 03 May 2012 10:04:04 GMTdescription changed
https://trac.sagemath.org/ticket/11768#comment:19
https://trac.sagemath.org/ticket/11768#comment:19
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11768?action=diff&version=19">diff</a>)
</li>
</ul>
<p>
With the second patch, the doctest becomes independent of whether or not <a class="closed ticket" href="https://trac.sagemath.org/ticket/12876" title="enhancement: Fix element and parent classes of Hom categories to be abstract, and ... (closed: fixed)">#12876</a> is applied.
</p>
<p>
Apply trac11768_source_of_dynamic_class.patch trac11768_docfix.patch
</p>
TicketSimonKingThu, 03 May 2012 10:06:56 GMT
https://trac.sagemath.org/ticket/11768#comment:20
https://trac.sagemath.org/ticket/11768#comment:20
<p>
Cool! I just notice:
</p>
<pre class="wiki">sage: C = Algebras(ZZ).TensorProducts()
sage: C.ParentMethods??
Type: classobj
String Form: sage.categories.algebras.Algebras.TensorProducts.ParentMethods
Namespace: Interactive
Loaded File: /mnt/local/king/SAGE/stable/sage-5.0.beta13/local/lib/python2.7/site-packages/sage/categories/algebras.py
Source File: /mnt/local/king/SAGE/stable/sage-5.0.beta13/devel/sage/sage/categories/algebras.py
Source:
class ParentMethods:
#def coproduct(self):
# tensor products of morphisms are not yet implemented
# return tensor(module.coproduct for module in self.modules)
pass
</pre><p>
I don't know who solved that, but (as I stated above) that used to fail!
</p>
TicketnthieryFri, 04 May 2012 20:22:18 GMT
https://trac.sagemath.org/ticket/11768#comment:21
https://trac.sagemath.org/ticket/11768#comment:21
<p>
By the way: feel free to steal this little fix which I did take the time not push through ...:
</p>
<p>
<a class="ext-link" href="http://combinat.sagemath.org/patches/file/tip/sage_inspect_for_classes-nt.patch"><span class="icon"></span>http://combinat.sagemath.org/patches/file/tip/sage_inspect_for_classes-nt.patch</a>
</p>
TicketSimonKingFri, 04 May 2012 21:30:14 GMT
https://trac.sagemath.org/ticket/11768#comment:22
https://trac.sagemath.org/ticket/11768#comment:22
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:21" title="Comment 21">nthiery</a>:
</p>
<blockquote class="citation">
<p>
By the way: feel free to steal this little fix which I did take the time not push through ...:
</p>
</blockquote>
<p>
??
</p>
<p>
According to the comments in the patch, it would <em>not</em> solve the problem addressed here:
</p>
<pre class="wiki">Now sage_getsourcelines works similarly for old and new style classes:
sage: sage_getsourcelines(Sets.ParentMethods)
sage: sage_getsourcelines(Sets.Algebras)
Still both fail, because inspect.getsourcelines (in fact
inspect.findsource) fails on nested classes which have been renamed
with NestedClassMetaclass:
...
</pre><p>
The point is that both <em>do</em> work with my patch:
</p>
<pre class="wiki">sage: from sage.misc.sageinspect import sage_getsourcelines
sage: for x in sage_getsourcelines(Sets.Algebras)[0]:
....: print x,
....:
class Algebras(AlgebrasCategory):
def extra_super_categories(self):
"""
EXAMPLES::
sage: Sets().Algebras(QQ).extra_super_categories()
[Category of modules with basis over Rational Field]
sage: Sets().Algebras(QQ).super_categories()
[Category of modules with basis over Rational Field]
sage: Sets().example().algebra(ZZ).categories()
[Category of set algebras over Integer Ring,
Category of modules with basis over Integer Ring,
...
Category of objects]
"""
from sage.categories.modules_with_basis import ModulesWithBasis
return [ModulesWithBasis(self.base_ring())]
</pre>
TicketnthierySat, 05 May 2012 07:25:26 GMT
https://trac.sagemath.org/ticket/11768#comment:23
https://trac.sagemath.org/ticket/11768#comment:23
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:22" title="Comment 22">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:21" title="Comment 21">nthiery</a>:
</p>
<blockquote class="citation">
<p>
By the way: feel free to steal this little fix which I did take the time not push through ...:
</p>
</blockquote>
<p>
According to the comments in the patch, it would <em>not</em> solve the problem addressed here:
</p>
<pre class="wiki">Now sage_getsourcelines works similarly for old and new style classes:
sage: sage_getsourcelines(Sets.ParentMethods)
sage: sage_getsourcelines(Sets.Algebras)
Still both fail, because inspect.getsourcelines (in fact
inspect.findsource) fails on nested classes which have been renamed
with NestedClassMetaclass:
...
</pre></blockquote>
<p>
Oh, sorry, I was unclear. I did not mean that this patch would fix
this ticket. Just that it fixed another little issue in a related spot
in the code. If your patch already takes care of it, so much the
better!
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingMon, 13 Aug 2012 13:08:16 GMT
https://trac.sagemath.org/ticket/11768#comment:24
https://trac.sagemath.org/ticket/11768#comment:24
<p>
Any review for better access to source code in the category framework?
</p>
TicketvbraunTue, 27 Nov 2012 10:45:54 GMT
https://trac.sagemath.org/ticket/11768#comment:25
https://trac.sagemath.org/ticket/11768#comment:25
<p>
Looks good. You might want to pick classes for doctests that are not moving targets in the combinat queue, though ;-) How about <code>sage/structure/sage_object.py</code> or something else thats buried deep down and unlikely to change.
</p>
TicketSimonKingTue, 27 Nov 2012 11:10:58 GMT
https://trac.sagemath.org/ticket/11768#comment:26
https://trac.sagemath.org/ticket/11768#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:25" title="Comment 25">vbraun</a>:
</p>
<blockquote class="citation">
<p>
Looks good. You might want to pick classes for doctests that are not moving targets in the combinat queue, though ;-) How about <code>sage/structure/sage_object.py</code> or something else thats buried deep down and unlikely to change.
</p>
</blockquote>
<p>
Good idea. I can not remember why I chose <code>JackPolynomials</code> to create examples. I'll see if <code>SageObject.element_class</code> shows the same problems with introspection than <code>JackPolynomials.element_class</code>.
</p>
TicketSimonKingTue, 27 Nov 2012 11:22:33 GMT
https://trac.sagemath.org/ticket/11768#comment:27
https://trac.sagemath.org/ticket/11768#comment:27
<p>
I think what I need is:
</p>
<ul><li>A parent class whose metaclass is a sub-class of <code>NestedClassMetaclass</code> (such as <code>UniqueRepresentation</code>)
</li><li>... so that the <code>Element</code> attribute is given as a nested class.
</li></ul><p>
These two properties hold for the <code>JackPolynomials</code> example.
</p>
<p>
What I could do is to create a minimal example in the doc tests. Unfortunately, I need a source file to make it work -- hence, such a doc test needs to use the cython(...) function and hence needs a compiler.
</p>
<p>
I did create a couple of examples of that type in other patches that got merged, but some people said it is bad practice to have examples that rely on a compiler.
</p>
<p>
So, what do you think? Shall I use the compiler, or perhaps create a new class in the Sage library just for the purpose of testing?
</p>
TicketSimonKingTue, 27 Nov 2012 11:27:30 GMT
https://trac.sagemath.org/ticket/11768#comment:28
https://trac.sagemath.org/ticket/11768#comment:28
<p>
I don't know yet whether my patch would fix the following example:
</p>
<pre class="wiki">sage: class MyParent(UniqueRepresentation, Parent):
....: class Element:
....: pass
....:
sage: P = MyParent(ZZ)
sage: P.element_class??
Type: DynamicMetaclass
Base Class: <class 'sage.structure.dynamic_class.DynamicMetaclass'>
String Form: <class '__main__.MyParent.element_class'>
Namespace: Interactive
Loaded File: /home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/IPython/FakeModule.py
Source File: /home/simon/SAGE/debug/sage-5.5.rc0/devel/sage/IPython/FakeModule.py
Source:
class DynamicMetaclass(type):
...
</pre><p>
If I understand correctly, my patch <em>would</em> fix a corresponding example in Cython (because it then has a source file to inspect). If my patch is able to fix the example above then I'll use it.
</p>
TicketSimonKingTue, 27 Nov 2012 16:46:08 GMT
https://trac.sagemath.org/ticket/11768#comment:29
https://trac.sagemath.org/ticket/11768#comment:29
<p>
To my surprise, the patch does <em>not</em> solve the introspection problem mentioned in my previous post, even if Cython code is used.
</p>
<p>
I think to make it work interactively should be moved on a different ticket. So, for now, I suggest to create a test class in sage.misc.sageinspect.
</p>
TicketSimonKingTue, 27 Nov 2012 20:26:52 GMTdescription changed
https://trac.sagemath.org/ticket/11768#comment:30
https://trac.sagemath.org/ticket/11768#comment:30
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11768?action=diff&version=30">diff</a>)
</li>
</ul>
<p>
I have folded the two patches into one, and I replaced some occurences of <code>trac ticket #...</code> by the appropriate <code>:trac:</code> directive.
</p>
<p>
And of course I removed the <code>JackPolynomial</code> test. Instead, the sources of a dummy class are inspected, created only for testing.
</p>
<p>
It turned out to be impossible to put the dummy class into <code>sage.misc.sageinspect</code>, since importing <code>UniqueRepresentation</code> and <code>Parent</code> in <code>sageinspect</code> seems to create an import cycle. Hence, the dummy is now in <code>sage.structure.parent</code> - which should be
</p>
<p>
Apply trac11768_source_of_dynamic_class.patch
</p>
TicketnthieryTue, 27 Nov 2012 20:58:10 GMT
https://trac.sagemath.org/ticket/11768#comment:31
https://trac.sagemath.org/ticket/11768#comment:31
<p>
Hi Simon!
</p>
<p>
Thanks for your work on finalizing this. What about putting the example in a new separate file.
What about putting the test class in a separate file like sage.misc.sageinspect_test.py that would only be imported in the doctest?
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingWed, 28 Nov 2012 10:16:29 GMT
https://trac.sagemath.org/ticket/11768#comment:32
https://trac.sagemath.org/ticket/11768#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:31" title="Comment 31">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Thanks for your work on finalizing this. What about putting the example in a new separate file.
What about putting the test class in a separate file like sage.misc.sageinspect_test.py that would only be imported in the doctest?
</p>
</blockquote>
<p>
Volker and Florent, do you agree with Nicolas that a separate file would be better?
</p>
TicketvbraunWed, 28 Nov 2012 10:41:46 GMT
https://trac.sagemath.org/ticket/11768#comment:33
https://trac.sagemath.org/ticket/11768#comment:33
<p>
If you can't find an appropriate class to test it with then I'm fine with an extra file. I would suggest something in <code>sage/tests</code>, though. Maybe <code>sage/tests/sageinspect.py</code>. The file can also test itself so it would really be a test (ooh my head hurts now).
</p>
TicketnthieryWed, 28 Nov 2012 20:34:30 GMT
https://trac.sagemath.org/ticket/11768#comment:34
https://trac.sagemath.org/ticket/11768#comment:34
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:33" title="Comment 33">vbraun</a>:
</p>
<blockquote class="citation">
<p>
If you can't find an appropriate class to test it with then I'm fine with an extra file. I would suggest something in <code>sage/tests</code>, though. Maybe <code>sage/tests/sageinspect.py</code>.
</p>
</blockquote>
<p>
I would argue for keeping the tests or test material close to the file under test. We already have sage.misc.nested_class_test which is very similar in purpose. Actually, maybe this should just go there!
</p>
<blockquote class="citation">
<p>
The file can also test itself so it would really be a test (ooh my head hurts now).
</p>
</blockquote>
<p>
:-)
</p>
TicketSimonKingWed, 28 Nov 2012 21:21:25 GMT
https://trac.sagemath.org/ticket/11768#comment:35
https://trac.sagemath.org/ticket/11768#comment:35
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11768#comment:34" title="Comment 34">nthiery</a>:
</p>
<blockquote class="citation">
<p>
I would argue for keeping the tests or test material close to the file under test. We already have sage.misc.nested_class_test which is very similar in purpose. Actually, maybe this should just go there!
</p>
</blockquote>
<p>
Good idea! I plan this for tomorrow.
</p>
TicketSimonKingThu, 29 Nov 2012 08:52:05 GMT
https://trac.sagemath.org/ticket/11768#comment:36
https://trac.sagemath.org/ticket/11768#comment:36
<p>
OK, done, ready for review!
</p>
<p>
Not that the coverage script complains. But I think the coverage script has a bug, since it reports:
</p>
<pre class="wiki">simon@linux-sqwp:~/SAGE/debug/sage-5.5.rc0/devel/sage> ../../sage -coverage sage/misc/sageinspect.py
----------------------------------------------------------------------
sage/misc/sageinspect.py
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE sage/misc/sageinspect.py: 60% (26 of 43)
Missing documentation:
* foo(x, a=')"', b={not (2+1==3):
* __init__(self):
* tokeneater(self, type, token, srow_scol, erow_ecol, line):
* foo(x, a=')"', b={(2+1):
* foo(x, a='\')"', b={not (2+1==3):
* dummy' + _grep_first_pair_of_parentheses(source) + ':\n return' try: return _sage_getargspec_from_ast(proxy) except SyntaxError: # To fix trac #10860. See #11913 for more information. return None elif isinstance(obj,functools.partial):
* test_funct(x,y):
* create_set_partition_function(letter, k):
* __cinit__(self):
* object pyobject(self):
* __init__(self, ring, gens, coerce=True):
* test1(a, b=2):
* test3(b, # 12 a=2):
* __init__(self, x=None, base=0):
* test1(a, b=2):
* test3(b, # 12 a=2):
Missing doctests:
* _getblock(lines):
Possibly wrong (function name doesn't occur in doctests):
* dummy' + source[beg:] + '\n return' return tuple(_sage_getargspec_from_ast(proxy)) except Exception: try: # Try to parse just the arguments as a Python argspec. proxy = 'def dummy' + _grep_first_pair_of_parentheses(source) + ':\n return' return tuple(_sage_getargspec_from_ast(proxy)) except Exception: raise ValueError, "Could not parse cython argspec" def sage_getfile(obj):
</pre><p>
First bug: It expects a <code>TestSuite.run()</code> test, but there is no class defined for which this would make sense.
</p>
<p>
Second bug: The <code>dummy' + source[beg:] + '\n return' return tuple(_sage_getargspec_from_ast(proxy))...</code> thingy.
</p>
<p>
Apply trac11768_source_of_dynamic_class.patch
</p>
TicketSimonKingFri, 30 Nov 2012 14:16:57 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/11768#comment:37
https://trac.sagemath.org/ticket/11768#comment:37
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>import location for nested test class</em>
</li>
</ul>
<p>
Oops, I just notice that in one test I still try to import the testclass from sage.structure.parent. Needs a rather trivial fix...
</p>
TicketSimonKingFri, 30 Nov 2012 14:49:36 GMTattachment set
https://trac.sagemath.org/ticket/11768
https://trac.sagemath.org/ticket/11768
<ul>
<li><strong>attachment</strong>
set to <em>trac11768_source_of_dynamic_class.patch</em>
</li>
</ul>
<p>
Get source lines of dynamic classes (e.g., parent_class of a category). Combined patch
</p>
TicketSimonKingFri, 30 Nov 2012 14:50:25 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/11768#comment:38
https://trac.sagemath.org/ticket/11768#comment:38
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>import location for nested test class</em> deleted
</li>
</ul>
<p>
Should now be fixed!
</p>
<p>
Apply trac11768_source_of_dynamic_class.patch
</p>
TicketvbraunSat, 01 Dec 2012 15:08:46 GMT
https://trac.sagemath.org/ticket/11768#comment:39
https://trac.sagemath.org/ticket/11768#comment:39
<p>
Looks good to me.
</p>
<p>
The coverage script is going crazy. But the correct fix would be to replace the regex cludge with a python lexer/parser. Definitely material for another ticket...
</p>
TicketvbraunSat, 01 Dec 2012 15:08:54 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/11768#comment:40
https://trac.sagemath.org/ticket/11768#comment:40
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
TicketjdemeyerFri, 21 Dec 2012 09:30:48 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/11768#comment:41
https://trac.sagemath.org/ticket/11768#comment:41
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.6.beta1</em>
</li>
</ul>
Ticket