Opened 11 years ago

Closed 11 years ago

#11954 closed defect (fixed)

improve the documentation of sage/misc/cython.py

Reported by: John Palmieri Owned by: Jason Grout
Priority: major Milestone: sage-5.0
Component: misc Keywords:
Cc: Merged in: sage-5.0.beta9
Authors: John Palmieri, David Loeffler Reviewers: David Loeffler, John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Various functions in cython.py have no documentation. The attached patch improves this. Note that after apply the patch, sage --coverage cython.py does not report 100%, because some multiline strings are interpreted as undocumented functions/classes/methods by the coverage script. Also, the function cython has no doctests. Suggests for that?

This also adds cython.py to the reference manual.

Attachments (2)

trac_11954-cython-doc.patch (12.7 KB) - added by John Palmieri 11 years ago.
trac_11954-review.patch (15.6 KB) - added by David Loeffler 11 years ago.
Apply over previous patch

Download all attachments as: .zip

Change History (12)

Changed 11 years ago by John Palmieri

Attachment: trac_11954-cython-doc.patch added

comment:1 Changed 11 years ago by John Palmieri

Status: newneeds_review

comment:2 Changed 11 years ago by John Palmieri

(This is a follow-up to #11887. In that ticket, we changed the documentation from the cython function from being completely empty to being "TODO: document this function!")

comment:3 in reply to:  description Changed 11 years ago by Leif Leonhardy

Replying to jhpalmieri:

sage --coverage cython.py does not report 100%, because some multiline strings are interpreted as undocumented functions/classes/methods by the coverage script.

Did you open a ticket for that? ;P

comment:4 Changed 11 years ago by John Palmieri

It might actually be covered by #7716, but I haven't tested it.

comment:5 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-4.7.3

Milestone sage-4.7.3 deleted

comment:6 Changed 11 years ago by David Loeffler

Milestone: sage-5.0
Reviewers: David Loeffler

Apply trac_11954-cython-doc.patch, trac_11954-review.patch

Here's a new patch which makes various cosmetic changes to the Sphinx formatting. I wasn't sure what the mysterious "sagobject_name" was supposed to mean in the docstring of the cython lambda function and the doctests weren't too helpful on that point; I'm guessing it's a typo for sage.object_name.

If you're happy with my reviewer patch, that's a positive review.

comment:7 Changed 11 years ago by John Palmieri

I'm mostly happy with the reviewer patch -- thanks for your work on this, it looks much better now. However, I don't like line 314:

'``Compiling <filename>...``'

The quotes don't look good after running Sphinx. Maybe it should be ``'Compiling <filename>...'``? Or maybe the single quotes should be dropped? Or drop the double back quotes instead?

Changed 11 years ago by David Loeffler

Attachment: trac_11954-review.patch added

Apply over previous patch

comment:8 Changed 11 years ago by David Loeffler

Apply trac_11954-cython-doc.patch, trac_11954-review.patch

This patch goes for the ``'foo'`` option.

comment:9 Changed 11 years ago by John Palmieri

Authors: John PalmieriJohn Palmieri, David Loeffler
Reviewers: David LoefflerDavid Loeffler, John Palmieri
Status: needs_reviewpositive_review

Great, looks good to me.

comment:10 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.0.beta9
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.