Opened 8 years ago
Closed 8 years ago
#11815 closed defect (fixed)
Embedding information in doc strings must not be formatted
Reported by: | SimonKing | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | misc | Keywords: | embedding format |
Cc: | vbraun, leif | Merged in: | sage-4.7.2.alpha3 |
Authors: | Simon King | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #9976 #11287 #11298 #11342 | Stopgaps: |
Description (last modified by )
A doc string from an extension module typically starts with embedding information, such as here:
sage: cython(""" ....: def testfunc(x): ....: 'some doc string' ....: return x ....: """) sage: print testfunc.__doc__ File: _mnt_local_king__sage_temp_mpc622_13356_tmp_0_spyx_0.pyx (starting at line 7) some doc string
There are two functions in sage.misc.sageinspect
that retrieve doc strings: sage_getdoc
and _sage_getdoc_unformatted
. The latter returns the doc string as it is, the former is supposed to remove the embedding information.
However, before doing so, sage_getdoc runs sage.misc.sagedoc.format
on the output of _sage_getdoc_unformatted
. That is a problem, because the formatting may insert line breaks, thereby destroying the embedding information, so that it can now not be correctly stripped from the doc string.
I suggest to modify sage.misc.sagedoc.format
, such that the first line is preserved, if it contains embedding information. In that case, directives are searched in the second line of the doc string, not in the first line (currently the only recognized directive is nodetex
).
Example, without the patch:
sage: from sage.misc.sagedoc import format sage: r = 'File: _local_user_with_a_very_long_name_that_would_normally_be_wrapped_sage_temp_machine_name_1234_tmp_1_spyx_0.pyx (starting at line 6)\nnodetex\nsome doc for a cython method\n`x \\geq y`' sage: print format(r) File: _local_user_with_a_very_long_name_that_would_normally_be_wrapped _sage_temp_machine_name_1234_tmp_1_spyx_0.pyx (starting at line 6) nodetex some doc for a cython method x >= y
The same example with the patch (it is a new doc test):
sage: r = 'File: _local_user_with_a_very_long_name_that_would_normally_be_wrapped_sage_temp_machine_name_1234_tmp_1_spyx_0.pyx (starting at line 6)\nnodetex\nsome doc for a cython method\n`x \\geq y`' sage: print format(r) File: _local_user_with_a_very_long_name_that_would_normally_be_wrapped_sage_temp_machine_name_1234_tmp_1_spyx_0.pyx (starting at line 6) <BLANKLINE> some doc for a cython method `x \geq y`
Apply trac11815_format_must_preserve_embedding.patch to the Sage library.
Attachments (1)
Change History (15)
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
Too bad, the patchbot starts with sage-4.7.1. But I do hope it works on top of alpha2. It definitely does on top of alpha3-prerelease.
comment:3 Changed 8 years ago by
- Dependencies set to #9976 #11298 #11342
I'll try whether it suffices to add some dependencies that are merged into alpha versions of 4.7.2
comment:4 Changed 8 years ago by
- Dependencies changed from #9976 #11298 #11342 to #9976 #11287 #11298 #11342
I think I know what dependency is missing: #11287. Trying again...
comment:5 follow-up: ↓ 6 Changed 8 years ago by
Replying to SimonKing:
The alpha3 tests pass modulo some numerical errors. I suppose that they are noise, unrelated with my patch.
Ooops, could you send me the failing ones, along with your system information (OS, processor, GCC version)?
comment:6 in reply to: ↑ 5 Changed 8 years ago by
Replying to leif:
Ooops, could you send me the failing ones, along with your system information (OS, processor, GCC version)?
I just did - off list, since I think it is not related with this ticket.
comment:7 follow-up: ↓ 9 Changed 8 years ago by
The good new is that now the patchbot manages to apply the patch.
The bad new is that the patchbot states:
The following tests failed: sage -t -force_lib devel/sage-11815/doc/fr/tutorial/programming.rst
The good news is that the patchbot is actually not showing any error in the log. So, why does it state that it fails?
comment:8 Changed 8 years ago by
- Status changed from needs_review to needs_work
- Work issues set to skip empty leading lines.
Actually, what I did ought to be improved: The problem is that a directive is not necessarily in the first line of the doc string after the embedding information:
sage: cython(""" ....: def testfunc(x): ....: ''' ....: nodetex ....: This is a doc string with raw latex ....: ....: `x \\geq y` ....: ''' ....: return -x ....: """) sage: print testfunc.__doc__ File: _mnt_local_king__sage_temp_mpc622_14073_tmp_0_spyx_0.pyx (starting at line 7) nodetex This is a doc string with raw latex `x \geq y`
Hence, one should strip empty lines at the beginning of a string resp. after the embedding information.
comment:9 in reply to: ↑ 7 Changed 8 years ago by
Replying to SimonKing:
The bad new is that the patchbot states:
The following tests failed: sage -t -force_lib devel/sage-11815/doc/fr/tutorial/programming.rst
The good news is that the patchbot is actually not showing any error in the log. So, why does it state that it fails?
Assuming the patchbot tests in parallel, it's likely to be an instance of the race conditions now fixed by #9739.
comment:10 Changed 8 years ago by
- Status changed from needs_work to needs_review
I updated the patch, and there is now a new doc test:
sage: cython_code = ["def testfunc(x):", ... " '''", ... " nodetex", ... " This is a doc string with raw latex", ... "", ... " `x \\geq y`", ... " '''", ... " return -x"] sage: cython('\n'.join(cython_code)) sage: from sage.misc.sageinspect import sage_getdoc sage: print sage_getdoc(testfunc) <BLANKLINE> This is a doc string with raw latex <BLANKLINE> `x \geq y` <BLANKLINE>
The trick is to strip leading empty lines (after the embedding information), and also to strip blank space around the directives.
comment:11 Changed 8 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
- Work issues skip empty leading lines. deleted
Applies fine to sage-4.7.2.alpha2 + #11342. Positive review.
comment:12 Changed 8 years ago by
I had forgotten to insert my name into the author list. Apart from that, there is no difference to the old patch. I hope it is OK if I keep it "positive review".
comment:13 Changed 8 years ago by
- Description modified (diff)
comment:14 Changed 8 years ago by
- Merged in set to sage-4.7.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
The attached patch was created on top of sage-4.7.2.alpha3-prerelease. I suppose that it will also apply on top of the official sage-4.7.2.alpha2. The alpha3 tests pass modulo some numerical errors. I suppose that they are noise, unrelated with my patch.
Hence, needs review!