Sage: Ticket #13916: Fix inspection of interactive Cython code
https://trac.sagemath.org/ticket/13916
<p>
The following happens with sage-5.6.b2, at least in the debug version from <a class="closed ticket" href="https://trac.sagemath.org/ticket/13864" title="task: Configure Python with pydebug when SAGE_DEBUG is set (closed: fixed)">#13864</a> (I could imagine that the new Cython version from <a class="closed ticket" href="https://trac.sagemath.org/ticket/13896" title="defect: Fix cython's gc_track and gc_untrack (closed: fixed)">#13896</a> could be related as well):
</p>
<pre class="wiki">sage: cython('''cpdef test_funct(x,y): return''')
sage: from sage.misc.sageinspect import sage_getfile
sage: os.path.exists(sage_getfile(test_funct))
False
</pre><p>
IIRC, it used to work. hence, I suppose some paths have changed.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/13916
Trac 1.1.6SimonKingSun, 06 Jan 2013 13:17:23 GMT
https://trac.sagemath.org/ticket/13916#comment:1
https://trac.sagemath.org/ticket/13916#comment:1
<p>
Could it be that simply Cython is writing the wrong information into the doc?
</p>
<pre class="wiki">sage: _sage_getdoc_unformatted(test_funct)
'File: _home_simon__sage_temp_linux_sqwp_site_6004_tmp_rJ1Nyc_spyx_0.pyx (starting at line 6)'
</pre><p>
But the file is at
</p>
<pre class="wiki">~/.sage/temp/linux_sqwp.site/6004/tmp_rJ1Nyc.spyx
</pre><p>
or
</p>
<pre class="wiki">~/.sage/temp/linux_sqwp.site/6004/spyx/_home_simon__sage_temp_linux_sqwp_site_6004_tmp_rJ1Nyc_spyx/tmp_rJ1Nyc.spyx
</pre><p>
(they coincide bit-wise). Note that even the ".pyx" information is wrong.
</p>
TicketSimonKingSun, 06 Jan 2013 13:22:11 GMTcc set
https://trac.sagemath.org/ticket/13916#comment:2
https://trac.sagemath.org/ticket/13916#comment:2
<ul>
<li><strong>cc</strong>
<em>robertwb</em> added
</li>
</ul>
TicketSimonKingSun, 06 Jan 2013 13:32:09 GMT
https://trac.sagemath.org/ticket/13916#comment:3
https://trac.sagemath.org/ticket/13916#comment:3
<p>
Actually there <em>is</em> a pyx file (I am in a new session now, hence, the names have changed):
</p>
<pre class="wiki">~/.sage/temp/linux_sqwp.site/6593/spyx/_home_simon__sage_temp_linux_sqwp_site_6593_tmp_mqjIHk_spyx/_home_simon__sage_temp_linux_sqwp_site_6593_tmp_mqjIHk_spyx_0.pyx
</pre><p>
The question is: Do we want to see the spyx file or the pyx file? Anyway, I think I'll manage to make sageinspect find that pyx file.
</p>
TicketSimonKingSun, 06 Jan 2013 13:57:17 GMTstatus changed; author set
https://trac.sagemath.org/ticket/13916#comment:4
https://trac.sagemath.org/ticket/13916#comment:4
<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>
With the attached patch, the following doctest works.
</p>
<pre class="wiki"> sage: cython('''cpdef test_funct(x,y): return''')
sage: print open(_extract_embedded_position(inspect.getdoc(test_funct))[1]).read()
<BLANKLINE>
include "interrupt.pxi" # ctrl-c interrupt block support
include "stdsage.pxi" # ctrl-c interrupt block support
<BLANKLINE>
include "cdefs.pxi"
cpdef test_funct(x,y): return
</pre><p>
The tests of sage/misc/sageinspect.py pass. Needs review!
</p>
TicketSnarkSun, 06 Jan 2013 14:40:32 GMT
https://trac.sagemath.org/ticket/13916#comment:5
https://trac.sagemath.org/ticket/13916#comment:5
<p>
First, about your patch:
</p>
<ol><li>it makes sage_getfile work on the box where I have doctest issues, which is positive ;
</li><li>but shouldn't os.path.join(SAGE_ROOT,'devel/sage', raw_filename) be os.path.join(SAGE_ROOT, 'devel', 'sage', raw_filename)? You're basically assuming that '/' is the path separator, which is perhaps a portability problem.
</li></ol><p>
On the box where I patched paths, the source file looks like this:
</p>
<pre class="wiki">
include "sage/ext/stdsage.pxi" # ctrl-c interrupt block support
include "cdefs.pxi"
cpdef test_funct(x,y): return
</pre><p>
while on another, unpatched, box (again, initial empty line is eaten by trac) it is:
</p>
<pre class="wiki">include "interrupt.pxi" # ctrl-c interrupt block support
include "stdsage.pxi" # ctrl-c interrupt block support
include "cdefs.pxi"
cpdef test_funct(x,y): return
</pre>
TicketSimonKingSun, 06 Jan 2013 15:09:30 GMT
https://trac.sagemath.org/ticket/13916#comment:6
https://trac.sagemath.org/ticket/13916#comment:6
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13916#comment:5" title="Comment 5">Snark</a>:
</p>
<blockquote class="citation">
<ol start="2"><li>but shouldn't os.path.join(SAGE_ROOT,'devel/sage', raw_filename) be os.path.join(SAGE_ROOT, 'devel', 'sage', raw_filename)? You're basically assuming that '/' is the path separator, which is perhaps a portability problem.
</li></ol></blockquote>
<p>
Yes, you are right. I'll update the patch in a minute.
</p>
<blockquote class="citation">
<p>
On the box where I patched paths, the source file looks like this:
</p>
<pre class="wiki">
include "sage/ext/stdsage.pxi" # ctrl-c interrupt block support
include "cdefs.pxi"
cpdef test_funct(x,y): return
</pre><p>
while on another, unpatched, box (again, initial empty line is eaten by trac) it is:
</p>
<pre class="wiki">include "interrupt.pxi" # ctrl-c interrupt block support
include "stdsage.pxi" # ctrl-c interrupt block support
include "cdefs.pxi"
cpdef test_funct(x,y): return
</pre></blockquote>
<p>
OK. That explains why the line number has changed, because the number of lines in front of the function definition has changed. And I suppose the change is indeed related with your patch from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a>, isn't it?
</p>
<p>
The patch from here is self-contained, but one old test and the new test from here will change with <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a>. Hence, I suggest to use this ticket as a new dependency for <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a>, and you add a patch on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a> that fixes the doctests.
</p>
TicketSimonKingSun, 06 Jan 2013 15:10:16 GMTattachment set
https://trac.sagemath.org/ticket/13916
https://trac.sagemath.org/ticket/13916
<ul>
<li><strong>attachment</strong>
set to <em>trac13916_inspect_interactive_cython.patch</em>
</li>
</ul>
TicketSimonKingSun, 06 Jan 2013 15:12:18 GMT
https://trac.sagemath.org/ticket/13916#comment:7
https://trac.sagemath.org/ticket/13916#comment:7
<p>
The patch is updated, still needs review.
</p>
TicketSnarkSun, 06 Jan 2013 16:00:07 GMT
https://trac.sagemath.org/ticket/13916#comment:8
https://trac.sagemath.org/ticket/13916#comment:8
<p>
I found a problem in my own patch, so this trac ticket is independent from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a>.
</p>
<p>
Simon, your patch looks nice, applies nice and compiles nice ; but since I'm going to get another round of rebuilding and full doctesting sage, I'll report what happens later (I'm not doing that on the arm box, so it shouldn't take two days :-P).
</p>
TicketSimonKingMon, 21 Jan 2013 10:49:37 GMT
https://trac.sagemath.org/ticket/13916#comment:9
https://trac.sagemath.org/ticket/13916#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13916#comment:8" title="Comment 8">Snark</a>:
</p>
<blockquote class="citation">
<p>
I found a problem in my own patch, so this trac ticket is independent from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12728" title="enhancement: sage's spkg sources should use correct include paths instead of having ... (closed: fixed)">#12728</a>.
</p>
<p>
Simon, your patch looks nice, applies nice and compiles nice ; but since I'm going to get another round of rebuilding and full doctesting sage, I'll report what happens later (I'm not doing that on the arm box, so it shouldn't take two days :-P).
</p>
</blockquote>
<p>
Now two weeks are over. Did it work?
</p>
TicketSimonKingMon, 21 Jan 2013 10:52:31 GMT
https://trac.sagemath.org/ticket/13916#comment:10
https://trac.sagemath.org/ticket/13916#comment:10
<p>
Since the coverage script complains, I'll try to add some new tests in sageinspect.py
</p>
TicketSimonKingMon, 21 Jan 2013 10:54:38 GMT
https://trac.sagemath.org/ticket/13916#comment:11
https://trac.sagemath.org/ticket/13916#comment:11
<p>
PS: The error reported by the patchbot seems unrelated.
</p>
TicketSimonKingMon, 21 Jan 2013 10:56:18 GMT
https://trac.sagemath.org/ticket/13916#comment:12
https://trac.sagemath.org/ticket/13916#comment:12
<p>
PPS: The coverage script reports nonsense. It says that the number of tests did not increase, but the number of functions did increase. But that's plain wrong: My patch does not introduce new functions, it only fixes existing functions. And it does add tests. So, I'll keep it like that, if you don't mind.
</p>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/13916#comment:13
https://trac.sagemath.org/ticket/13916#comment:13
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/13916#comment:14
https://trac.sagemath.org/ticket/13916#comment:14
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TicketmmezzarobbaFri, 14 Mar 2014 10:29:16 GMTcommit, branch set
https://trac.sagemath.org/ticket/13916#comment:15
https://trac.sagemath.org/ticket/13916#comment:15
<ul>
<li><strong>commit</strong>
set to <em>69169ab524e3273bfa36cad94dc735fe80cf029b</em>
</li>
<li><strong>branch</strong>
set to <em>u/mmezzarobba/13916-inspect_interactive_cython</em>
</li>
</ul>
<p>
Didn't apply cleanly to 6.0+; updated; needs review again.
</p>
TicketvbraunMon, 07 Apr 2014 21:11:16 GMTkeywords, reviewer set
https://trac.sagemath.org/ticket/13916#comment:16
https://trac.sagemath.org/ticket/13916#comment:16
<ul>
<li><strong>keywords</strong>
<em>days57</em> added
</li>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
<p>
Looks good to me
</p>
TicketvbraunMon, 07 Apr 2014 21:13:07 GMTstatus changed
https://trac.sagemath.org/ticket/13916#comment:17
https://trac.sagemath.org/ticket/13916#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketvbraunTue, 08 Apr 2014 10:17:37 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/13916#comment:18
https://trac.sagemath.org/ticket/13916#comment:18
<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>branch</strong>
changed from <em>u/mmezzarobba/13916-inspect_interactive_cython</em> to <em>69169ab524e3273bfa36cad94dc735fe80cf029b</em>
</li>
</ul>
Ticket