Sage: Ticket #8513: Including documentation in the reference manual for some files related to graph theory
https://trac.sagemath.org/ticket/8513
<p>
I noticed a few weeks ago while reviewing a patch that important files in the graph theory folder were not appearing anywhere in the reference manual. For instance, functions such as <code>vertex_cut</code> or <code>edge_cut</code> do not appear, as well as all functions defined only for directed graphs.
</p>
<p>
At that time, I thought about adding the missing files with the patch, but since there were a lot of warnings displayed by Sphinx while generating the documentation, I changed my mind.
</p>
<p>
I think it would be a good idea to use this ticket to fix this, but since it touches many files of graph theory, it may be hard to do it in a clean way. Someone has an idea of what would be the best approach ?
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/8513
Trac 1.1.6mvnguSat, 13 Mar 2010 00:33:09 GMT
https://trac.sagemath.org/ticket/8513#comment:1
https://trac.sagemath.org/ticket/8513#comment:1
<p>
Wait until Sage 4.3.4 is released. I expect that release to fix all the warnings relating to building the HTML and PDF versions of the Sage standard documentation. At least, I expect that release to fix such warnings for the reference manual.
</p>
TicketabmasseMon, 22 Mar 2010 17:00:52 GMTstatus, cc changed
https://trac.sagemath.org/ticket/8513#comment:2
https://trac.sagemath.org/ticket/8513#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_work</em>
</li>
<li><strong>cc</strong>
<em>rlm</em> added
</li>
</ul>
<p>
Here are the warnings I get on my computer:
</p>
<pre class="wiki">[~/Applications/sage/devel/sage-combinat/doc/en/reference]$ sage -docbuild reference html
sphinx-build -b html -d /Users/alexandre/Applications/sage/devel/sage/doc/output/doctrees/en/reference /Users/alexandre/Applications/sage/devel/sage/doc/en/reference /Users/alexandre/Applications/sage/devel/sage/doc/output/html/en/reference
Running Sphinx v0.6.3
loading pickled environment... done
building [html]: targets for 612 source files that are out of date
updating environment: 2 added, 612 changed, 0 removed
reading sources... [100%] sage/symbolic/ring curve_morphismtorcfield
/Users/alexandre/Applications/sage/local/lib/python2.6/site-packages/sage/graphs/generic_graph.py:docstring of sage.graphs.generic_graph:3: (ERROR/3) Unexpected indentation.
/Users/alexandre/Applications/sage/local/lib/python2.6/site-packages/sage/graphs/generic_graph.py:docstring of sage.graphs.generic_graph.GenericGraph.minimum_outdegree_orientation:7: (WARNING/2) Block quote ends without a blank line; unexpected unindent.
/Users/alexandre/Applications/sage/local/lib/python2.6/site-packages/sage/graphs/generic_graph.py:docstring of sage.graphs.generic_graph.GenericGraph.spanning_trees_count:18: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.
/Users/alexandre/Applications/sage/local/lib/python2.6/site-packages/sage/graphs/generic_graph.py:docstring of sage.graphs.generic_graph.GenericGraph.spanning_trees_count:27: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst:7926: (WARNING/2) Duplicate explicit target name: "krg96".
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst:7930: (WARNING/2) Duplicate explicit target name: "gyll93".
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst:7926: WARNING: duplicate citation KRG96, other instance in /Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst:7930: WARNING: duplicate citation GYLL93, other instance in /Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/graphs/generic_graph.rst
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] structure urve_morphismtorcfield
writing additional files... genindex modindex search
copying images... media/homology/rp2.png media/homology/simplices.png media/homology/torus.png media/homology/klein.png media/homology/torus_labelled.png
copying static files... done
dumping search index... done
dumping object inventory... done
build succeeded, 8 warnings.
Build finished. The built documents can be found in /Users/alexandre/Applications/sage/devel/sage/doc/output/html/en/reference
</pre><p>
I'll try to fix them and upload a patch soon.
</p>
TicketabmasseMon, 22 Mar 2010 19:49:21 GMTattachment set
https://trac.sagemath.org/ticket/8513
https://trac.sagemath.org/ticket/8513
<ul>
<li><strong>attachment</strong>
set to <em>trac_8513_graph_theory_documentation-abm.patch</em>
</li>
</ul>
<p>
Adds digraph.py and generic_graph.py in the doctree
</p>
TicketabmasseMon, 22 Mar 2010 19:51:35 GMTstatus, priority changed
https://trac.sagemath.org/ticket/8513#comment:3
https://trac.sagemath.org/ticket/8513#comment:3
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>priority</strong>
changed from <em>major</em> to <em>minor</em>
</li>
</ul>
<p>
Finally, since there weren't too many warnings, I think it's worth trying to fix all of them. This is done in the patch I just submitted.
</p>
<p>
To summarize, this patch adds the two files digraph.py and generic_graph.py to the reference manual and correct the less than 10 warnings that Sphinx was displaying.
</p>
<p>
Needs review!
</p>
TicketncohenTue, 23 Mar 2010 10:54:58 GMT
https://trac.sagemath.org/ticket/8513#comment:4
https://trac.sagemath.org/ticket/8513#comment:4
<p>
Excellent ! Now we appear in the doc, and there are no warnings anymore.
</p>
<p>
Reviewing your patch, I met a few things that needed fixing in the docs, so if you agree with my modifications, you can set this to positive review.. Thank you !! :-)
</p>
<p>
Nathann
</p>
TicketabmasseTue, 23 Mar 2010 13:12:21 GMT
https://trac.sagemath.org/ticket/8513#comment:5
https://trac.sagemath.org/ticket/8513#comment:5
<p>
I agree with your changes ! I just hope it won't overlap with any other ticket.
</p>
<p>
By the way, did you test my patch ? When I created it, I used the sage-combinat queue and it worked fine, but here, when I tried to test your patch, I created a clone (with <code>hg -clone t8513</code>) to apply your patch on top of mine, but I got the following warning.
</p>
<pre class="wiki">[~/Applications/sage/devel/sage-t8513]$ sage -docbuild reference html
sphinx-build -b html -d /Users/alexandre/Applications/sage/devel/sage/doc/output/doctrees/en/reference /Users/alexandre/Applications/sage/devel/sage/doc/en/reference /Users/alexandre/Applications/sage/devel/sage/doc/output/html/en/reference
Running Sphinx v0.6.3
loading pickled environment... done
building [html]: targets for 2 source files that are out of date
updating environment: 0 added, 2 changed, 0 removed
reading sources... [100%] sage/graphs/graph
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/graphs.rst:4: (WARNING/2) toctree references unknown document u'sage/graphs/digraph'
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/graphs.rst:4: (WARNING/2) toctree references unknown document u'sage/graphs/generic_graph'
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] sage/graphs/graph
writing additional files... genindex modindex search
copying static files... done
dumping search index... done
dumping object inventory... done
build succeeded, 2 warnings.
</pre><p>
It's strange, it doesn't accept to add the two files to the doctree, as if the path was not correct when creating a clone, but was ok with sage-combinat. Does it work fine for you?
</p>
TicketncohenTue, 23 Mar 2010 13:34:44 GMT
https://trac.sagemath.org/ticket/8513#comment:6
https://trac.sagemath.org/ticket/8513#comment:6
<p>
O_o
</p>
<p>
Odd.... I applied your patch as usual, and I mean by this not using sage-combinat. It applied fine, and passed all tests. After my modifications, same result O_o
</p>
<p>
Have you tried cloning the branch using sage -clone instead of doing it manually with hg ?
</p>
<p>
Nathann
</p>
TicketabmasseTue, 23 Mar 2010 14:08:03 GMT
https://trac.sagemath.org/ticket/8513#comment:7
https://trac.sagemath.org/ticket/8513#comment:7
<p>
No idea... Anyway, I'll try it with the sage-combinat queue to look at the documentation generated by your patch and it should be fine.
</p>
TicketmvnguMon, 12 Apr 2010 19:59:48 GMTattachment set
https://trac.sagemath.org/ticket/8513
https://trac.sagemath.org/ticket/8513
<ul>
<li><strong>attachment</strong>
set to <em>trac_8513_graph_theory_documentation-smallfixes.patch</em>
</li>
</ul>
TicketmvnguMon, 12 Apr 2010 20:01:58 GMT
https://trac.sagemath.org/ticket/8513#comment:8
https://trac.sagemath.org/ticket/8513#comment:8
<p>
I have replaced ncohen's patch with one that has the ticket number in its commit message.
</p>
TicketjhpalmieriMon, 12 Apr 2010 21:50:06 GMT
https://trac.sagemath.org/ticket/8513#comment:9
https://trac.sagemath.org/ticket/8513#comment:9
<p>
I don't know much about graph theory, but is there any sense to the current order in the reference manual? Wouldn't it make more sense for <code>graph</code> to come first, for example, rather than <code>cliquer</code>? I don't think alphabetical is the right approach: someone who wants to know about graphs in Sage may very well start at the first link in the "Graph Theory" chapter. (It might be even better to have an introductory section in the file devel/sage/doc/en/reference/graphs.rst (like matrices.rst, for example). That probably belongs on another ticket, though.)
</p>
TicketabmasseMon, 12 Apr 2010 22:02:23 GMT
https://trac.sagemath.org/ticket/8513#comment:10
https://trac.sagemath.org/ticket/8513#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8513#comment:9" title="Comment 9">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I don't know much about graph theory, but is there any sense to the current order in the reference manual? Wouldn't it make more sense for <code>graph</code> to come first, for example, rather than <code>cliquer</code>? I don't think alphabetical is the right approach: someone who wants to know about graphs in Sage may very well start at the first link in the "Graph Theory" chapter. (It might be even better to have an introductory section in the file devel/sage/doc/en/reference/graphs.rst (like matrices.rst, for example). That probably belongs on another ticket, though.)
</p>
</blockquote>
<p>
I agree with you! There is some cleanup needed in the documentation. But maybe we should wait for it in another ticket. The goal of <a class="closed ticket" href="https://trac.sagemath.org/ticket/8513" title="defect: Including documentation in the reference manual for some files related ... (closed: fixed)">#8513</a> is to quickly make the graph documentation available so that one may check if Sphinx builds all right when reviewing related tickets. If it's ok for you, of course.
</p>
<p>
Sorry for being so late, by the way, Nathann, I'll try to finish this ticket today or at most, during the week.
</p>
TicketmvnguMon, 12 Apr 2010 22:18:55 GMTreviewer, author set
https://trac.sagemath.org/ticket/8513#comment:11
https://trac.sagemath.org/ticket/8513#comment:11
<ul>
<li><strong>reviewer</strong>
set to <em>Minh Van Nguyen</em>
</li>
<li><strong>author</strong>
set to <em>Alexandre Blondin Massé, Nathann Cohen</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8513#comment:9" title="Comment 9">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I don't know much about graph theory, but is there any sense to the current order in the reference manual?
</p>
</blockquote>
<p>
No, I don't think so. The current organization is a mess.
<br /><br />
</p>
<blockquote class="citation">
<p>
Wouldn't it make more sense for <code>graph</code> to come first, for example, rather than <code>cliquer</code>?
</p>
</blockquote>
<p>
Yes. The current organization of the graph theory modules, as they appear in the reference manual, is rather unusual. For example, the interface to cliquer appears as the very first link. One would expect something along the following line to be more natural:
</p>
<ul><li>undirected graph
</li><li>digraph
</li><li>generic graph
</li><li>applications of graph theory
</li><li>fast compiled graph
</li><li>etc.
</li></ul><p>
I have uploaded a reviewer patch that does something about this. With my reviewer patch, the organization of the graph theory modules should be more systematic.
<br /><br />
</p>
<blockquote class="citation">
<p>
I don't think alphabetical is the right approach: someone who wants to know about graphs in Sage may very well start at the first link in the "Graph Theory" chapter. (It might be even better to have an introductory section in the file devel/sage/doc/en/reference/graphs.rst (like matrices.rst, for example). That probably belongs on another ticket, though.)
</p>
</blockquote>
<p>
Nod. This needs to wait for another ticket.
<br /><br />
</p>
<p>
After applying <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/8513/trac_8513_graph_theory_documentation-abm.patch"><span class="icon"></span>trac_8513_graph_theory_documentation-abm.patch</a> and <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/8513/trac_8513_graph_theory_documentation-smallfixes.patch"><span class="icon"></span>trac_8513_graph_theory_documentation-smallfixes.patch</a>, I then rebuilt the whole Sage documentation. A long doctest of the whole Sage library resulted in the following failure:
</p>
<pre class="wiki">sage -t -long devel/sage-main/sage/misc/sagedoc.py
**********************************************************************
File "/dev/shm/mvngu/sandbox/sage-4.3.5-8513-graph-doc/devel/sage-main/sage/misc/sagedoc.py", line 892:
sage: len(search_doc('tree', whole_word=True, interact=False).splitlines()) < 100
Expected:
True
Got:
False
</pre><p>
This is due to adding two new files to the reference manual that happen to be about graph theory, hence the above search returns more matches for the word "tree". My reviewer patch should fix this doctest failure. It needs reviewing by anyone but me. I'm happy with both abmasse and ncohen's patches. Apply patches in the following order:
</p>
<ol><li><a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/8513/trac_8513_graph_theory_documentation-abm.patch"><span class="icon"></span>trac_8513_graph_theory_documentation-abm.patch</a>
</li><li><a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/8513/trac_8513_graph_theory_documentation-smallfixes.patch"><span class="icon"></span>trac_8513_graph_theory_documentation-smallfixes.patch</a>
</li><li><a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/8513/trac_8513-reviewer.patch"><span class="icon"></span>trac_8513-reviewer.patch</a>
</li></ol>
TicketabmasseMon, 12 Apr 2010 22:51:28 GMT
https://trac.sagemath.org/ticket/8513#comment:12
https://trac.sagemath.org/ticket/8513#comment:12
<p>
I'm running the longtest on all sage right now. Giving some feedback later tonight.
</p>
TicketjhpalmieriMon, 12 Apr 2010 23:25:17 GMTstatus, reviewer changed
https://trac.sagemath.org/ticket/8513#comment:13
https://trac.sagemath.org/ticket/8513#comment:13
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Minh Van Nguyen</em> to <em>Minh Van Nguyen, John Palmieri</em>
</li>
</ul>
<blockquote class="citation">
<p>
My reviewer patch should fix this doctest failure. It needs reviewing by anyone but me.
</p>
</blockquote>
<p>
Your patch fixes the problem, but it doesn't really fit in sagedoc.py: the doctests there are supposed to contrast what happens if <code>whole_word=True</code> is not used (more than 2000 matches) vs. when it is (used to be fewer than 100 matches). How about changing it to <code>len(...) < 150</code> or <code>len(...) < 200</code>?
</p>
TicketmvnguMon, 12 Apr 2010 23:32:02 GMTattachment set
https://trac.sagemath.org/ticket/8513
https://trac.sagemath.org/ticket/8513
<ul>
<li><strong>attachment</strong>
set to <em>trac_8513-reviewer.patch</em>
</li>
</ul>
<p>
reviewer patch
</p>
TicketmvnguMon, 12 Apr 2010 23:35:31 GMTstatus changed
https://trac.sagemath.org/ticket/8513#comment:14
https://trac.sagemath.org/ticket/8513#comment:14
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8513#comment:13" title="Comment 13">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
Your patch fixes the problem, but it doesn't really fit in sagedoc.py: the doctests there are supposed to contrast what happens if <code>whole_word=True</code> is not used (more than 2000 matches) vs. when it is (used to be fewer than 100 matches). How about changing it to <code>len(...) < 150</code> or <code>len(...) < 200</code>?
</p>
</blockquote>
<p>
I see what you mean. Performing a search with <code>whole_word=False</code> results in too many matches. When using <code>whole_word=True</code>, we want to provide an upper bound on the number of matches. I have modified the reviewer patch accordingly.
</p>
TicketjhpalmieriTue, 13 Apr 2010 02:56:36 GMT
https://trac.sagemath.org/ticket/8513#comment:15
https://trac.sagemath.org/ticket/8513#comment:15
<p>
I'm happy with the reviewer patch now. After rebuilding the docs, all tests pass on sage.math. Does that mean the whole ticket gets a positive review?
</p>
TicketabmasseTue, 13 Apr 2010 13:55:07 GMT
https://trac.sagemath.org/ticket/8513#comment:16
https://trac.sagemath.org/ticket/8513#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8513#comment:15" title="Comment 15">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I'm happy with the reviewer patch now. After rebuilding the docs, all tests pass on sage.math. Does that mean the whole ticket gets a positive review?
</p>
</blockquote>
<p>
Sure, if you're ok with the result. It's even better if the final reviewer isn't Nathann nor me.
</p>
TicketmvnguTue, 13 Apr 2010 23:33:03 GMT
https://trac.sagemath.org/ticket/8513#comment:17
https://trac.sagemath.org/ticket/8513#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8513#comment:15" title="Comment 15">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I'm happy with the reviewer patch now. After rebuilding the docs, all tests pass on sage.math. Does that mean the whole ticket gets a positive review?
</p>
</blockquote>
<p>
I think so. Cleaning up the modules added by this ticket to the reference manual can be split off to an enhancement ticket.
</p>
TicketjhpalmieriWed, 14 Apr 2010 05:24:03 GMTstatus changed
https://trac.sagemath.org/ticket/8513#comment:18
https://trac.sagemath.org/ticket/8513#comment:18
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/8684" title="defect: clean up graph theory documentation in the reference manual (closed: fixed)">#8684</a> for a follow-up ticket, the purpose of which is to organize the graph theory stuff in the reference manual. I'm not going to do this myself, since I don't know enough about graph theory in general, or the graph theory in Sage, to know how to organize it.
</p>
TicketjhpalmieriFri, 16 Apr 2010 18:46:01 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/8513#comment:19
https://trac.sagemath.org/ticket/8513#comment:19
<ul>
<li><strong>status</strong>
changed from <em>positive_review</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.4.alpha0</em>
</li>
</ul>
<p>
Merged in 4.4.alpha0:
</p>
<ul><li>trac_8513_graph_theory_documentation-abm.patch
</li><li>trac_8513_graph_theory_documentation-smallfixes.patch
</li><li>trac_8513-reviewer.patch
</li></ul>
Ticket