Opened 6 years ago
Closed 5 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) | 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 6 years ago by
comment:2 Changed 6 years ago by
- Cc robertwb added
comment:3 Changed 6 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 6 years ago by
- Status changed from new to 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 6 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 in reply to: ↑ 5 Changed 6 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 6 years ago by
comment:7 Changed 6 years ago by
The patch is updated, still needs review.
comment:8 follow-up: ↓ 9 Changed 6 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 in reply to: ↑ 8 Changed 6 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 6 years ago by
Since the coverage script complains, I'll try to add some new tests in sageinspect.py
comment:11 Changed 6 years ago by
PS: The error reported by the patchbot seems unrelated.
comment:12 Changed 6 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 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:14 Changed 5 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:15 Changed 5 years ago by
- Branch set to u/mmezzarobba/13916-inspect_interactive_cython
- Commit set to 69169ab524e3273bfa36cad94dc735fe80cf029b
Didn't apply cleanly to 6.0+; updated; needs review again.
comment:16 Changed 5 years ago by
- Keywords days57 added
- Reviewers set to Volker Braun
Looks good to me
comment:17 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:18 Changed 5 years ago by
- Branch changed from u/mmezzarobba/13916-inspect_interactive_cython to 69169ab524e3273bfa36cad94dc735fe80cf029b
- Resolution set to fixed
- Status changed from positive_review to 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.