Fix inspection of interactive Cython code

[simon-king-jena]

The following happens with sage-5.6.b2, at least in the debug version from #13864 (I could imagine that the new Cython version from #13896 could be related as well):

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

IIRC, it used to work. hence, I suppose some paths have changed.

CC: @robertwb

Component: misc

Keywords: days57

Author: Simon King

Branch/Commit: 69169ab

Reviewer: Volker Braun

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

[simon-king-jena]

comment:1

Could it be that simply Cython is writing the wrong information into the doc?

sage: _sage_getdoc_unformatted(test_funct)
'File: _home_simon__sage_temp_linux_sqwp_site_6004_tmp_rJ1Nyc_spyx_0.pyx (starting at line 6)'

But the file is at

~/.sage/temp/linux_sqwp.site/6004/tmp_rJ1Nyc.spyx

or

~/.sage/temp/linux_sqwp.site/6004/spyx/_home_simon__sage_temp_linux_sqwp_site_6004_tmp_rJ1Nyc_spyx/tmp_rJ1Nyc.spyx

(they coincide bit-wise). Note that even the ".pyx" information is wrong.

[simon-king-jena]

comment:3

Actually there is a pyx file (I am in a new session now, hence, the names have changed):

~/.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

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.

[simon-king-jena]

comment:4

With the attached patch, the following doctest works.

        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

The tests of sage/misc/sageinspect.py pass. Needs review!

[simon-king-jena]

Author: Simon King

[SnarkBoojum]

comment:5

First, about your patch:

1. it makes sage_getfile work on the box where I have doctest issues, which is positive ;
2. 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.

On the box where I patched paths, the source file looks like this:

include "sage/ext/stdsage.pxi"  # ctrl-c interrupt block support

include "cdefs.pxi"
cpdef test_funct(x,y): return

while on another, unpatched, box (again, initial empty line is eaten by trac) it is:

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

[simon-king-jena]

comment:6

Replying to @SnarkBoojum:

2. 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.

Yes, you are right. I'll update the patch in a minute.

On the box where I patched paths, the source file looks like this:

include "sage/ext/stdsage.pxi"  # ctrl-c interrupt block support

include "cdefs.pxi"
cpdef test_funct(x,y): return

while on another, unpatched, box (again, initial empty line is eaten by trac) it is:

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

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 #12728, isn't it?

The patch from here is self-contained, but one old test and the new test from here will change with #12728. Hence, I suggest to use this ticket as a new dependency for #12728, and you add a patch on #12728 that fixes the doctests.

[simon-king-jena]

Attachment: trac13916_inspect_interactive_cython.patch.gz

[simon-king-jena]

comment:7

The patch is updated, still needs review.

[SnarkBoojum]

comment:8

I found a problem in my own patch, so this trac ticket is independent from #12728.

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).

[simon-king-jena]

comment:9

Replying to @SnarkBoojum:

I found a problem in my own patch, so this trac ticket is independent from #12728.

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).

Now two weeks are over. Did it work?

[simon-king-jena]

comment:10

Since the coverage script complains, I'll try to add some new tests in sageinspect.py

[simon-king-jena]

comment:11

PS: The error reported by the patchbot seems unrelated.

[simon-king-jena]

comment:12

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.

[mezzarobba]

Branch: u/mmezzarobba/13916-inspect_interactive_cython

[mezzarobba]

comment:15

Didn't apply cleanly to 6.0+; updated; needs review again.

[mezzarobba]

Commit: 69169ab

[vbraun]

Changed keywords from none to days57

[vbraun]

Reviewer: Volker Braun

[vbraun]

comment:16

Looks good to me

[vbraun]

Changed branch from u/mmezzarobba/13916-inspect_interactive_cython to 69169ab