Ticket #4902 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

[with patch, positive review] convert sage.algebras.* docstrings to Sphinx

Reported by: mhansen Owned by: tba
Priority: major Milestone: sage-3.4
Component: documentation Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description


Attachments

trac_4902.patch Download (185.4 KB) - added by mhansen 4 years ago.
trac_4902-2.patch Download (6.9 KB) - added by mhansen 4 years ago.
sage.algebras-final.patch Download (184.5 KB) - added by mhansen 4 years ago.

Change History

Changed 4 years ago by mhansen

comment:1 Changed 4 years ago by mhansen

  • Summary changed from convert sage.algebras.* docstrings to Sphinx to [with patch, needs review] convert sage.algebras.* docstrings to Sphinx

comment:2 Changed 4 years ago by jhpalmieri

  • Summary changed from [with patch, needs review] convert sage.algebras.* docstrings to Sphinx to [with patch, needs work] convert sage.algebras.* docstrings to Sphinx

Mostly looks good. I have one major issue, and then some little things:

One major problem: I notice that methods starting with _ seem to be missing from the html version of the documentation (e.g., __cmp__ and __init__). These were present in the the old version; was there a reason for their omission? Because of this omission, I'm not going to be able to review the changes to the docstrings of such methods (because I'm not familiar enough with ReST, and so I'm using both the source code and the html to do the review).

Minor things:

free_algebra.py, lines 165-167 (a blank line, then ::, then a blank line in the docstring for FreeAlgebra_generic): these can be deleted, I think, since they needlessly break the EXAMPLES into two sections.

quaternion_algebra.py, line 134. Why in the html version does the line

def QuaternionAlgebra(K, a, b, names=['i','j','k'], denom=1): 

end up showing a comma after the left bracket? It looks like names=[, 'i', 'j', 'k'].

quaternion_algebra.py, line 321: in the html, the string `\mathbb{Z}[i]`, can allow a line break between Z[i] and the comma. Can this be avoided?

quaternion_algebra_element.py, line 219: need a line break and an empty line between "Do we?" and "EXAMPLES::"

quaternion_algebra_element.py, line 234: need a line break and an empty line between "scalar part..." and "EXAMPLES::"

steenrod_algebra_bases.py, lines 121-122: I don't think that "INTERNAL DOCUMENTATION" needs to be a section heading, or whatever it is now. Maybe change this to "INTERNAL DOCUMENTATION::" and delete the line of hyphens following it?

steenrod_algebra_bases.py, lines 128-149: each line starting with "add" is supposed to be an item in a list.

steenrod_algebra_bases.py, lines 1394-1395: `t`th iterated commutator of consecutive `\text{Sq}^{2^i}` 's. isn't being rendered properly. Maybe change `t`th to `t^{th}`?

The same thing happens on line 1417 with `n`th.

steenrod_algebra_bases.py, line 1462: the original text said the cache _steenrod_bases is set to {} before doing the computations., but the {} disappeared in the new version.

steenrod_algebra_element.py, lines 11-12: EXAMPLES should have a double colon after it, and the line of hyphens below it should be deleted.

steenrod_algebra_element.py, lines 272-273: as above, perhaps INTERNAL DOCUMENTATION should have :: after it, and no line of hyphens below it.

steenrod_algebra_element.py, line 1518: change `i`th to `i^{th}`

steenrod_algebra_element.py, line 1530: change `k`th to `k^{th}`

Changed 4 years ago by mhansen

comment:3 follow-up: ↓ 5 Changed 4 years ago by mhansen

I've posted a patch which takes care of a number of the issues you noted.

The problem with "def QuaternionAlgebra?" is internal to Sphinx. I'll send a message to their mailing list.

Currently, the documentation for "internal" methods that start with "_" are not included in the reference manual by default. In the future, I have some ideas for extending Sphinx's autodoc extension to be able to include things underscore methods without cluttering things up for users not interested in them.

comment:4 Changed 4 years ago by mhansen

  • Summary changed from [with patch, needs work] convert sage.algebras.* docstrings to Sphinx to [with patch, needs review] convert sage.algebras.* docstrings to Sphinx

comment:5 in reply to: ↑ 3 Changed 4 years ago by jhpalmieri

Replying to mhansen:

I've posted a patch which takes care of a number of the issues you noted.

Great, I'll take a look soon.

Currently, the documentation for "internal" methods that start with "_" are not included in the reference manual by default. In the future, I have some ideas for extending Sphinx's autodoc extension to be able to include things underscore methods without cluttering things up for users not interested in them.

From Sage's point of view, various classes have a lot of documentation in the init method, so unfortunately leaving them out means leaving out some important things. Anyway...

comment:6 Changed 4 years ago by jhpalmieri

  • Summary changed from [with patch, needs review] convert sage.algebras.* docstrings to Sphinx to [with patch, positive review] convert sage.algebras.* docstrings to Sphinx

Looks good, positive review. You also caught some things that I had missed, or maybe you were fixing typos in the original documentation. Thanks.

Changed 4 years ago by mhansen

comment:7 Changed 4 years ago by mabshoff

  • Status changed from new to closed
  • Resolution set to fixed

Merged in Sage 3.4.alpha0.

Cheers,

Michael

Note: See TracTickets for help on using tickets.