Sage: Ticket #6840: [with patch, positive review] Fix documentation for Sage Notebook
https://trac.sagemath.org/ticket/6840
<p>
Change documentation of the notebook to a uniform format and complete notebook doctest/documentation coverage.
</p>
<ul><li>Some docstrings are improperly formatted.
</li><li>The formatting of docstrings is inconsistent, e.g.:
</li></ul><p>
Some have:
</p>
<pre class="wiki">Output: footype
Description
</pre><p>
While others have:
</p>
<pre class="wiki">Output:
- footype -- description
</pre><p>
etc.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/6840
Trac 1.1.6timdumolSat, 29 Aug 2009 07:31:56 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation.patch</em>
</li>
</ul>
<p>
Fixes docstrings added by patch <a class="closed ticket" href="https://trac.sagemath.org/ticket/6568" title="enhancement: [with patch, positive review] Migrate Notebook to Jinja (closed: fixed)">#6568</a> in <code>notebook.py</code> and <code>worksheet.py</code>. Depends on <a class="closed ticket" href="https://trac.sagemath.org/ticket/6568" title="enhancement: [with patch, positive review] Migrate Notebook to Jinja (closed: fixed)">#6568</a>
</p>
TicketmpatelSun, 30 Aug 2009 10:16:59 GMT
https://trac.sagemath.org/ticket/6840#comment:1
https://trac.sagemath.org/ticket/6840#comment:1
<p>
I think there is some "overlap" here with version 2 of the patch at <a class="closed ticket" href="https://trac.sagemath.org/ticket/5360" title="defect: Re-reading of #4927 convert sage.server.* docstrings to Sphinx (closed: duplicate)">#5360</a>. But that patch makes only a few changes to <code>notebook.py</code>. To avoid merge conflicts, should I put them in a separate patch or bring them here?
</p>
TickettimdumolSun, 30 Aug 2009 16:54:21 GMT
https://trac.sagemath.org/ticket/6840#comment:2
https://trac.sagemath.org/ticket/6840#comment:2
<p>
If you don't mind, I merged the changes from <a class="closed ticket" href="https://trac.sagemath.org/ticket/5360" title="defect: Re-reading of #4927 convert sage.server.* docstrings to Sphinx (closed: duplicate)">#5360</a> to save you the trouble.
</p>
TicketmpatelSun, 30 Aug 2009 18:09:44 GMT
https://trac.sagemath.org/ticket/6840#comment:3
https://trac.sagemath.org/ticket/6840#comment:3
<p>
Thanks! <a class="closed ticket" href="https://trac.sagemath.org/ticket/5360" title="defect: Re-reading of #4927 convert sage.server.* docstrings to Sphinx (closed: duplicate)">#5360</a> is now marked for closure.
</p>
TickettimdumolSun, 30 Aug 2009 18:11:38 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation-v2.patch</em>
</li>
</ul>
<p>
Merged changes from <a class="closed ticket" href="https://trac.sagemath.org/ticket/5360" title="defect: Re-reading of #4927 convert sage.server.* docstrings to Sphinx (closed: duplicate)">#5360</a>. Apply only this file.
</p>
TickettimdumolSun, 30 Aug 2009 18:13:32 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation.v2.patch</em>
</li>
</ul>
<p>
OBSOLETE.
</p>
TicketmpatelSun, 30 Aug 2009 18:18:08 GMT
https://trac.sagemath.org/ticket/6840#comment:4
https://trac.sagemath.org/ticket/6840#comment:4
<p>
I think the patch somehow got doubled in size during the merger.
</p>
TickettimdumolSun, 30 Aug 2009 19:31:30 GMT
https://trac.sagemath.org/ticket/6840#comment:5
https://trac.sagemath.org/ticket/6840#comment:5
<p>
Yes, it got messed up the first times around, but it seems to be the right size: <code>trac_6840-notebook-documentation-v2.patch (112.6 KB)</code>.
</p>
TicketmpatelMon, 31 Aug 2009 02:52:49 GMT
https://trac.sagemath.org/ticket/6840#comment:6
https://trac.sagemath.org/ticket/6840#comment:6
<p>
Version 3 adds <em>lots</em> of small changes to <code>introspect.py</code>, <code>misc.py</code>, <code>docHTMLProcessor.py</code>, <code>interact.py</code>, <code>notebook.py</code>, <code>template.py</code>, and <code>support.py</code>.
</p>
<p>
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 <code>worksheet.py</code>.
</p>
<p>
I apologize for stepping on any toes. There may still be other problems, besides those I just introduced. Prepping <code>server/</code> for the reference manual is tedious, but the docstrings do appear to render somewhat more nicely now, especially those in <code>interact.py</code>.
</p>
<p>
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.
</p>
TicketmpatelMon, 31 Aug 2009 03:08:17 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation-v3.patch</em>
</li>
</ul>
<p>
Version 3. Apply only this patch.
</p>
TickettimdumolMon, 31 Aug 2009 10:15:45 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation-v4.patch</em>
</li>
</ul>
<p>
Version 4. Brought up <code>cell.py</code> coverage to almost 100%, except for 2 functions.
</p>
TickettimdumolMon, 31 Aug 2009 10:18:46 GMTsummary changed
https://trac.sagemath.org/ticket/6840#comment:7
https://trac.sagemath.org/ticket/6840#comment:7
<ul>
<li><strong>summary</strong>
changed from <em>Fix documentation for Sage Notebook</em> to <em>[with patch, needs review]Fix documentation for Sage Notebook</em>
</li>
</ul>
<p>
Version 4 adds docstrings and doctests for <code>cell.py</code> except for 2 functions: one of which is a hack (<code>doc_html</code>), and the other I have no idea how to test (<code>stop_interacting</code>).
</p>
<p>
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?).
</p>
TicketmvnguMon, 31 Aug 2009 12:52:56 GMTsummary changed
https://trac.sagemath.org/ticket/6840#comment:8
https://trac.sagemath.org/ticket/6840#comment:8
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs review]Fix documentation for Sage Notebook</em> to <em>[with patch, needs work]Fix documentation for Sage Notebook</em>
</li>
</ul>
<p>
The patch <code>trac_6840-notebook-documentation-v4.patch</code> applies OK against 4.1.1, but with one fuzz:
</p>
<pre class="wiki">[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
</pre><p>
Building the reference manual results in about 10 warnings:
</p>
<pre class="wiki">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.
</pre><p>
The test suite reports one doctest failure:
</p>
<pre class="wiki">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]
</pre><p>
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.
</p>
TickettimdumolMon, 31 Aug 2009 14:32:30 GMTsummary changed
https://trac.sagemath.org/ticket/6840#comment:9
https://trac.sagemath.org/ticket/6840#comment:9
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs work]Fix documentation for Sage Notebook</em> to <em>[with patch, needs review]Fix documentation for Sage Notebook</em>
</li>
</ul>
<p>
I've fixed the warnings and the doctest failure.
</p>
TickettimdumolMon, 31 Aug 2009 14:43:55 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation-v5.patch</em>
</li>
</ul>
<p>
Fixed documentation warnings and doctest failure. Apply this patch only.
</p>
TicketmpatelMon, 31 Aug 2009 20:43:37 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-notebook-documentation-v6.patch</em>
</li>
</ul>
<p>
Miscellaneous tiny changes (see comment). Apply only this patch.
</p>
TicketmpatelMon, 31 Aug 2009 20:54:01 GMTsummary changed; author set
https://trac.sagemath.org/ticket/6840#comment:10
https://trac.sagemath.org/ticket/6840#comment:10
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs review]Fix documentation for Sage Notebook</em> to <em>[with patch, needs review] Fix documentation for Sage Notebook</em>
</li>
<li><strong>author</strong>
set to <em>Tim Dumol, Mitesh Patel</em>
</li>
</ul>
<p>
v6 changes:
</p>
<ul><li>Fixed <code>INPUT::</code> in <code>template.py</code>.
</li><li>Removed the space immediately after <code>:meth:</code> in two places in <code>cell.py</code>.
</li><li>Removed <code>config.py</code> from <code>notebook.rst</code>, since there's no significant documentation yet on notebook key bindings.
</li></ul><p>
That's impressive coverage for <code>cell.py</code>! It seems that <code>Cell.stop_interacting()</code> isn't even called in the Sage library.
</p>
TicketmpatelMon, 31 Aug 2009 21:04:53 GMT
https://trac.sagemath.org/ticket/6840#comment:11
https://trac.sagemath.org/ticket/6840#comment:11
<p>
One way to placate Sphinx about the unused source file <code>doc/en/reference/sage/server/notebook/config.rst</code>: Delete <code>doc/output/*/en/reference/</code> and rebuild from scratch.
</p>
TicketmvnguTue, 01 Sep 2009 04:20:04 GMT
https://trac.sagemath.org/ticket/6840#comment:12
https://trac.sagemath.org/ticket/6840#comment:12
<p>
The patch <code>trac_6840-notebook-documentation-v6.patch</code> applies OK, but with fuzz:
</p>
<pre class="wiki">[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
</pre><p>
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:
</p>
<ol><li><code>trac_6840-notebook-documentation-v6.patch</code>
</li><li><code>trac_6840-reviewer.patch</code>
</li></ol>
TicketmpatelTue, 01 Sep 2009 05:16:36 GMT
https://trac.sagemath.org/ticket/6840#comment:13
https://trac.sagemath.org/ticket/6840#comment:13
<p>
Thanks very much for fixing the typos, including <code>AttributeErrro</code>.
</p>
<p>
Is it alright if we treat the "PLANNING" section near the top of <code>interact.py</code> 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 (<code>2D</code> to <code>2-D</code>).
</p>
<p>
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?
</p>
TicketmpatelTue, 01 Sep 2009 05:25:19 GMT
https://trac.sagemath.org/ticket/6840#comment:14
https://trac.sagemath.org/ticket/6840#comment:14
<p>
While I'm here I should ask whether we should put both side-effects, including possible exceptions, <em>and</em> return values in <code>OUTPUT</code> sections. Mostly, I've mentioned just the return values in <code>OUTPUT</code>. On occasion, I've noted significant effects in the blurb at the top of a docstring.
</p>
TicketmvnguTue, 01 Sep 2009 05:35:14 GMTattachment set
https://trac.sagemath.org/ticket/6840
https://trac.sagemath.org/ticket/6840
<ul>
<li><strong>attachment</strong>
set to <em>trac_6840-reviewer.patch</em>
</li>
</ul>
<p>
apply on top of previous patch
</p>
TicketmvnguTue, 01 Sep 2009 05:42:18 GMT
https://trac.sagemath.org/ticket/6840#comment:15
https://trac.sagemath.org/ticket/6840#comment:15
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/6840#comment:13" title="Comment 13">mpatel</a>:
</p>
<blockquote class="citation">
<p>
Is it alright if we treat the "PLANNING" section near the top of <code>interact.py</code> as "internal" documentation, at least for now? I haven't checked that it's current and Sphinx complains copiously about it.
</p>
</blockquote>
<p>
New reviewer patch attached. Please use that one. That should get rid of the complaints by Sphinx.
</p>
<p>
<br />
</p>
<blockquote class="citation">
<blockquote>
<p>
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 (<code>2D</code> to <code>2-D</code>).
</p>
</blockquote>
</blockquote>
<p>
The updated reviewer patch also addresses this issue.
</p>
<p>
<br />
</p>
<blockquote class="citation">
<p>
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?
</p>
</blockquote>
<p>
I don't know.
</p>
<p>
<br />
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/6840#comment:14" title="Comment 14">mpatel</a>:
</p>
<blockquote class="citation">
<p>
While I'm here I should ask whether we should put both side-effects, including possible exceptions, <em>and</em> return values in <code>OUTPUT</code> sections. Mostly, I've mentioned just the return values in <code>OUTPUT</code>. On occasion, I've noted significant effects in the blurb at the top of a docstring.
</p>
</blockquote>
<p>
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.
</p>
TicketmpatelTue, 01 Sep 2009 06:05:50 GMTsummary, author changed; reviewer set
https://trac.sagemath.org/ticket/6840#comment:16
https://trac.sagemath.org/ticket/6840#comment:16
<ul>
<li><strong>reviewer</strong>
set to <em>Minh Van Nguyen, Mitesh Patel</em>
</li>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] Fix documentation for Sage Notebook</em> to <em>[with patch, positive review] Fix documentation for Sage Notebook</em>
</li>
<li><strong>author</strong>
changed from <em>Tim Dumol, Mitesh Patel</em> to <em>Tim Dumol, Minh Van Nguyen, Mitesh Patel</em>
</li>
</ul>
TicketmvnguTue, 01 Sep 2009 06:12:17 GMTstatus, author changed; resolution, merged set
https://trac.sagemath.org/ticket/6840#comment:17
https://trac.sagemath.org/ticket/6840#comment:17
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>Sage 4.1.2.alpha0</em>
</li>
<li><strong>author</strong>
changed from <em>Tim Dumol, Minh Van Nguyen, Mitesh Patel</em> to <em>Tim Dumol, Mitesh Patel</em>
</li>
</ul>
<p>
Merged patches in this order:
</p>
<ol><li><code>trac_6840-notebook-documentation-v6.patch</code>
</li><li><code>trac_6840-reviewer.patch</code>
</li></ol><p>
I take reviewer credit, but not authorship credit :-)
</p>
Ticket