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

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)

trac11815_format_must_preserve_embedding.patch (4.7 KB) - added by SimonKing 8 years ago.
Preserve embedding information when formatting a raw doc string

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by SimonKing

  • Status changed from new to needs_review

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!

comment:2 Changed 8 years ago by SimonKing

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 SimonKing

  • 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 SimonKing

  • 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: Changed 8 years ago by leif

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 SimonKing

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: Changed 8 years ago by SimonKing

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 SimonKing

  • 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 leif

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 SimonKing

  • 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 vbraun

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

Changed 8 years ago by SimonKing

Preserve embedding information when formatting a raw doc string

comment:12 Changed 8 years ago by SimonKing

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 leif

  • Description modified (diff)

comment:14 Changed 8 years ago by leif

  • Merged in set to sage-4.7.2.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.