Opened 10 years ago
Closed 9 years ago
#13916 closed defect (fixed)
Fix inspection of interactive Cython code
Reported by: | SimonKing | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-6.2 |
Component: | misc | Keywords: | days57 |
Cc: | robertwb | Merged in: | |
Authors: | Simon King | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | 69169ab (Commits, GitHub, GitLab) | Commit: | 69169ab524e3273bfa36cad94dc735fe80cf029b |
Dependencies: | Stopgaps: |
Description
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.
Attachments (1)
Change History (19)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Cc: | robertwb added |
---|
comment:3 Changed 10 years ago by
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.
comment:4 Changed 10 years ago by
Authors: | → Simon King |
---|---|
Status: | new → needs_review |
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!
comment:5 follow-up: 6 Changed 10 years ago by
First, about your patch:
- it makes sage_getfile work on the box where I have doctest issues, which is positive ;
- 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
comment:6 Changed 10 years ago by
Replying to Snark:
- 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): returnwhile 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.
Changed 10 years ago by
Attachment: | trac13916_inspect_interactive_cython.patch added |
---|
comment:8 follow-up: 9 Changed 10 years ago by
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).
comment:9 Changed 10 years ago by
Replying to Snark:
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?
comment:10 Changed 10 years ago by
Since the coverage script complains, I'll try to add some new tests in sageinspect.py
comment:12 Changed 10 years ago by
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.
comment:13 Changed 9 years ago by
Milestone: | sage-5.11 → sage-5.12 |
---|
comment:14 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:15 Changed 9 years ago by
Branch: | → u/mmezzarobba/13916-inspect_interactive_cython |
---|---|
Commit: | → 69169ab524e3273bfa36cad94dc735fe80cf029b |
Didn't apply cleanly to 6.0+; updated; needs review again.
comment:17 Changed 9 years ago by
Status: | needs_review → positive_review |
---|
comment:18 Changed 9 years ago by
Branch: | u/mmezzarobba/13916-inspect_interactive_cython → 69169ab524e3273bfa36cad94dc735fe80cf029b |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Could it be that simply Cython is writing the wrong information into the doc?
But the file is at
or
(they coincide bit-wise). Note that even the ".pyx" information is wrong.