Opened 9 years ago
Closed 9 years ago
#6840 closed defect (fixed)
[with patch, positive review] Fix documentation for Sage Notebook
Reported by: | timdumol | Owned by: | boothby |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.1.2 |
Component: | notebook | Keywords: | notebook documentation |
Cc: | Merged in: | Sage 4.1.2.alpha0 | |
Authors: | Tim Dumol, Mitesh Patel | Reviewers: | Minh Van Nguyen, Mitesh Patel |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Change documentation of the notebook to a uniform format and complete notebook doctest/documentation coverage.
- Some docstrings are improperly formatted.
- The formatting of docstrings is inconsistent, e.g.:
Some have:
Output: footype Description
While others have:
Output: - footype -- description
etc.
Attachments (8)
Change History (25)
Changed 9 years ago by
comment:1 Changed 9 years ago by
I think there is some "overlap" here with version 2 of the patch at #5360. But that patch makes only a few changes to notebook.py
. To avoid merge conflicts, should I put them in a separate patch or bring them here?
comment:2 Changed 9 years ago by
If you don't mind, I merged the changes from #5360 to save you the trouble.
comment:3 Changed 9 years ago by
Thanks! #5360 is now marked for closure.
comment:4 Changed 9 years ago by
I think the patch somehow got doubled in size during the merger.
comment:5 Changed 9 years ago by
Yes, it got messed up the first times around, but it seems to be the right size: trac_6840-notebook-documentation-v2.patch (112.6 KB)
.
comment:6 Changed 9 years ago by
Version 3 adds lots of small changes to introspect.py
, misc.py
, docHTMLProcessor.py
, interact.py
, notebook.py
, template.py
, and support.py
.
Topics: missing docstrings, directives, blank lines, bullet points, headings; other inconsistencies; Sphinx warnings; etc. The doctests pass, apart the known problem with randomly ordered tests in worksheet.py
.
I apologize for stepping on any toes. There may still be other problems, besides those I just introduced. Prepping server/
for the reference manual is tedious, but the docstrings do appear to render somewhat more nicely now, especially those in interact.py
.
Please feel free to make more changes. Or we can just put this up for review and fix the remaining quirks as we find them.
Changed 9 years ago by
Version 4. Brought up cell.py
coverage to almost 100%, except for 2 functions.
comment:7 Changed 9 years ago by
- Summary changed from Fix documentation for Sage Notebook to [with patch, needs review]Fix documentation for Sage Notebook
Version 4 adds docstrings and doctests for cell.py
except for 2 functions: one of which is a hack (doc_html
), and the other I have no idea how to test (stop_interacting
).
I'll put this up for review, and we can work on the rest of the documentation another time (unless you want to add some more now?).
comment:8 Changed 9 years ago by
- Summary changed from [with patch, needs review]Fix documentation for Sage Notebook to [with patch, needs work]Fix documentation for Sage Notebook
The patch trac_6840-notebook-documentation-v4.patch
applies OK against 4.1.1, but with one fuzz:
[mvngu@sage sage-main]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6840/trac_6840-notebook-documentation-v4.patch && hg qpush adding trac_6840-notebook-documentation-v4.patch to series file applying trac_6840-notebook-documentation-v4.patch patching file sage/server/notebook/config.py Hunk #1 succeeded at 1 with fuzz 1 (offset -1 lines). Now at: trac_6840-notebook-documentation-v4.patch
Building the reference manual results in about 10 warnings:
WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.Cell.html:8: (WARNING/2) Inline literal start-string without end-string. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.Cell.html_out:6: (WARNING/2) Bullet list ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.Cell.is_interacting:8: (WARNING/2) Block quote ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.Cell.output_text:10: (WARNING/2) Bullet list ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.Cell.parse_html:15: (WARNING/2) Block quote ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.ComputeCell.html:8: (WARNING/2) Inline literal start-string without end-string. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.ComputeCell.html_out:6: (WARNING/2) Bullet list ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.ComputeCell.is_interacting:8: (WARNING/2) Block quote ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.ComputeCell.output_text:10: (WARNING/2) Bullet list ends without a blank line; unexpected unindent. WARNING: /scratch/mvngu/release/sage-4.1.1/local/lib/python2.6/site-packages/sage/server/notebook/cell.py:docstring of sage.server.notebook.cell.ComputeCell.parse_html:15: (WARNING/2) Block quote ends without a blank line; unexpected unindent.
The test suite reports one doctest failure:
sage -t -long devel/sage-main/sage/server/notebook/cell.py ********************************************************************** File "/scratch/mvngu/release/sage-4.1.1/devel/sage-main/sage/server/notebook/cell.py", line 1598: sage: C.introspect_html() Expected: '<div class="docstring">\n \n <p><span class="math">foobar</span></p>\n\n\n</div>' Got: '<div class="docstring">\n \n \n <p><span class="math">foobar</span></p>\n\n\n</div>' ********************************************************************** 1 items had failures: 1 of 13 in __main__.example_71 ***Test Failed*** 1 failures. For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.1/tmp/.doctest_cell.py [26.4 s]
I'm marking this as "needs work", since at least the doctest failure must be resolved. The warnings from building the reference manual can be addressed via another ticket. However, it would be nice if both of these issues could be fixed here.
comment:9 Changed 9 years ago by
- Summary changed from [with patch, needs work]Fix documentation for Sage Notebook to [with patch, needs review]Fix documentation for Sage Notebook
I've fixed the warnings and the doctest failure.
comment:10 Changed 9 years ago by
- Summary changed from [with patch, needs review]Fix documentation for Sage Notebook to [with patch, needs review] Fix documentation for Sage Notebook
v6 changes:
- Fixed
INPUT::
intemplate.py
. - Removed the space immediately after
:meth:
in two places incell.py
. - Removed
config.py
fromnotebook.rst
, since there's no significant documentation yet on notebook key bindings.
That's impressive coverage for cell.py
! It seems that Cell.stop_interacting()
isn't even called in the Sage library.
comment:11 Changed 9 years ago by
One way to placate Sphinx about the unused source file doc/en/reference/sage/server/notebook/config.rst
: Delete doc/output/*/en/reference/
and rebuild from scratch.
comment:12 Changed 9 years ago by
The patch trac_6840-notebook-documentation-v6.patch
applies OK, but with fuzz:
[mvngu@sage sage-main]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6840/trac_6840-notebook-documentation-v6.patch && hg qpush adding trac_6840-notebook-documentation-v6.patch to series file applying trac_6840-notebook-documentation-v6.patch patching file sage/server/notebook/config.py Hunk #1 succeeded at 1 with fuzz 1 (offset -1 lines). Now at: trac_6840-notebook-documentation-v6.patch
Don't worry too much about the fuzz. I have attached a reviewer patch that fixes typos introduced by the previous patch. If you're OK with that reviewer patch, then the ticket gets a positive review and patches should be merged in this order:
trac_6840-notebook-documentation-v6.patch
trac_6840-reviewer.patch
comment:13 Changed 9 years ago by
Thanks very much for fixing the typos, including AttributeErrro
.
Is it alright if we treat the "PLANNING" section near the top of interact.py
as "internal" documentation, at least for now? I haven't checked that it's current and Sphinx complains copiously about it. Either way, just let me know, and I'll attach v2 of the reviewer patch, since I found at least one more minor change to make (2D
to 2-D
).
By the way, is there a reST alternative to double back-quotes that puts the enclosed text in a fixed-width font but does not change its background color?
comment:14 follow-up: ↓ 15 Changed 9 years ago by
While I'm here I should ask whether we should put both side-effects, including possible exceptions, and return values in OUTPUT
sections. Mostly, I've mentioned just the return values in OUTPUT
. On occasion, I've noted significant effects in the blurb at the top of a docstring.
comment:15 in reply to: ↑ 14 Changed 9 years ago by
Replying to mpatel:
Is it alright if we treat the "PLANNING" section near the top of
interact.py
as "internal" documentation, at least for now? I haven't checked that it's current and Sphinx complains copiously about it.
New reviewer patch attached. Please use that one. That should get rid of the complaints by Sphinx.
Either way, just let me know, and I'll attach v2 of the reviewer patch, since I found at least one more minor change to make (
2D
to2-D
).
The updated reviewer patch also addresses this issue.
By the way, is there a reST alternative to double back-quotes that puts the enclosed text in a fixed-width font but does not change its background color?
I don't know.
Replying to mpatel:
While I'm here I should ask whether we should put both side-effects, including possible exceptions, and return values in
OUTPUT
sections. Mostly, I've mentioned just the return values inOUTPUT
. On occasion, I've noted significant effects in the blurb at the top of a docstring.
That's a good idea. Document what works and what doesn't. I usually have a "TESTS:" section for a method/function/class where I document exceptions that could be raised when using that method/function/class.
comment:16 Changed 9 years ago by
- Reviewers set to Minh Van Nguyen, Mitesh Patel
- Summary changed from [with patch, needs review] Fix documentation for Sage Notebook to [with patch, positive review] Fix documentation for Sage Notebook
comment:17 Changed 9 years ago by
- Merged in set to Sage 4.1.2.alpha0
- Resolution set to fixed
- Status changed from new to closed
Merged patches in this order:
trac_6840-notebook-documentation-v6.patch
trac_6840-reviewer.patch
I take reviewer credit, but not authorship credit :-)
Fixes docstrings added by patch #6568 in
notebook.py
andworksheet.py
. Depends on #6568