Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve the documentation of sage/misc/cython.py #11954

Closed
jhpalmieri opened this issue Oct 25, 2011 · 14 comments
Closed

improve the documentation of sage/misc/cython.py #11954

jhpalmieri opened this issue Oct 25, 2011 · 14 comments
Assignees
Milestone

Comments

@jhpalmieri
Copy link
Member

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.

Component: misc

Author: John Palmieri, David Loeffler

Reviewer: David Loeffler, John Palmieri

Merged: sage-5.0.beta9

Issue created by migration from https://trac.sagemath.org/ticket/11954

@jhpalmieri
Copy link
Member Author

comment:1

Attachment: trac_11954-cython-doc.patch.gz

@jhpalmieri
Copy link
Member Author

comment:2

(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!")

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 26, 2011

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

@jhpalmieri
Copy link
Member Author

comment:4

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

@jdemeyer
Copy link

jdemeyer commented Nov 3, 2011

Milestone sage-4.7.3 deleted

@jdemeyer jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 13, 2012

comment:6

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.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 13, 2012

Reviewer: David Loeffler

@loefflerd loefflerd mannequin added this to the sage-5.0 milestone Mar 13, 2012
@jhpalmieri
Copy link
Member Author

comment:7

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 ...'``? Or maybe the single quotes should be dropped? Or drop the double back quotes instead?

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 13, 2012

Attachment: trac_11954-review.patch.gz

Apply over previous patch

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 13, 2012

comment:8

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

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

@jhpalmieri
Copy link
Member Author

Changed reviewer from David Loeffler to David Loeffler, John Palmieri

@jhpalmieri
Copy link
Member Author

comment:9

Great, looks good to me.

@jhpalmieri
Copy link
Member Author

Changed author from John Palmieri to John Palmieri, David Loeffler

@jdemeyer
Copy link

Merged: sage-5.0.beta9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants