Opened 2 years ago
Closed 2 years ago
#12816 closed enhancement (fixed)
Documentation and list of Graph functions
Reported by: | ncohen | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | graph theory | Keywords: | |
Cc: | dcoudert | Merged in: | sage-5.1.beta1 |
Authors: | Nathann Cohen | Reviewers: | David Coudert |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12743, #12903 | Stopgaps: |
Description (last modified by ncohen)
This patch only touches documentation. It mainly adds lists of function to the top of important Graph files so that we can, at long long long last, see what on earth those objects can do. This list will have to be maintained manually, so this is a bad solution to a problem that should be fixed by Sphinx. Only it is not, and we really need to have such lists !
Apply:
Nathann
Attachments (1)
Change History (28)
comment:1 Changed 2 years ago by ncohen
- Status changed from new to needs_review
comment:2 Changed 2 years ago by dcoudert
- Status changed from needs_review to needs_info
comment:3 Changed 2 years ago by dcoudert
- Reviewers set to David Coudert
comment:4 Changed 2 years ago by ncohen
Done here ! But I would really like to get a good answer from the Sphinx guys, for the html file produced really is awful :-p
https://groups.google.com/d/topic/sphinx-dev/GWLtoAHGOo4/discussion
Nathann
comment:5 Changed 2 years ago by dcoudert
In the produced html files, you can see that:
- The width of columns is proportional to the number of '=' per column
- The width of columns of table in same file is consistent: 45-55, 54-46, ...
- However, the width of the table is never indicated. If you add for instance in file graph.html a "width=100%" option to each table directives, the rendering is much better (columns in different tables have same width.
Unfortunately, I don't think sphinx allows for adding such directive :(
This is exactly the same story with the other type of tables (+----------+---------+).
comment:6 Changed 2 years ago by dcoudert
A better solution is to use csv-table (http://docutils.sourceforge.net/docs/ref/rst/directives.html#table). Bellow is an example for graph.py
**Graph basic operations:** .. csv-table:: :class: contentstable :header: "Methods", "Description" :widths: 50, 50 :delim: | :meth:`~Graph.write_to_eps` | Writes a plot of the graph to ``filename`` in ``eps`` format. :meth:`~Graph.to_undirected` | Since the graph is already undirected, simply returns a copy of itself. :meth:`~Graph.to_directed` | Returns a directed version of the graph. :meth:`~Graph.sparse6_string` | Returns the sparse6 representation of the graph as an ASCII string. :meth:`~Graph.graph6_string` | Returns the graph6 representation of the graph as an ASCII string. :meth:`~Graph.bipartite_sets` | Returns `(X,Y)` where X and Y are the nodes in each bipartite set of graph. :meth:`~Graph.bipartite_color` | Returns a dictionary with vertices as the keys and the color class as the values. :meth:`~Graph.is_directed` | Since graph is undirected, returns False. **Distances:** .. csv-table:: :class: contentstable :header: "Methods", "Description" :widths: 50, 50 :delim: | :meth:`~Graph.centrality_closeness` | Returns the closeness centrality (1/average distance to all vertices) :meth:`~Graph.centrality_degree` | Returns the degree centrality :meth:`~Graph.centrality_betweenness` | Returns the betweenness centrality **Graph properties:** .. csv-table:: :class: contentstable :header: "Methods", "Description" :widths: 50, 50 :delim: | :meth:`~Graph.is_prime` | Tests whether the current graph is prime. :meth:`~Graph.is_split` | Returns ``True`` if the graph is a Split graph, ``False`` otherwise. :meth:`~Graph.is_triangle_free` | Returns whether ``self`` is triangle-free :meth:`~Graph.is_bipartite` | Returns True if graph G is bipartite, False if not. :meth:`~Graph.is_line_graph` | Tests wether the graph is a line graph. :meth:`~Graph.is_odd_hole_free` | Tests whether ``self`` contains an induced odd hole. :meth:`~Graph.is_even_hole_free` | Tests whether ``self`` contains an induced even hole.
The output is better, but we have another issue to solve.
In the original graph.html file produced when building the documentation, we have:
- <table border="1" class="docutils">
Using the :class: contentstable directive, I obtain:
- <table border="1" class="contentstable docutils">
But then the produced table is of width 90%. Also, the ultimate solution would be to add in file doc/output/html/en/reference/_static/basic.css an entry like
table.fullwidthtable { width: 100%; }
and then to use the directive :class: fullwidthtable (or table100). Other options like center can also be added.
However, I don't know how to add such stuff into the basic.css file...
I think this is the best way to do it. Furthermore, it is much easier to use/maintain than the simple table solution.
D.
comment:7 Changed 2 years ago by ncohen
- Description modified (diff)
- Status changed from needs_info to needs_review
Patch updated ! Honestly, I am pretty happy with this result. Thank you for this contentstable trick !!
I dedicate this patch to Emacs' macro feature, without which none of this would have been possible.
God I love those macros :-D
Nathann
comment:8 follow-up: ↓ 9 Changed 2 years ago by dcoudert
Escape-Meta-Alt-Control-Shift is great ;-)
comment:9 in reply to: ↑ 8 Changed 2 years ago by ncohen
Escape-Meta-Alt-Control-Shift is great ;-)
It has become an illness, actually. My shortcut to shut my computer down is "CTRL + ALT + SHIFT + HOME" since last week. For the moment it looks likle this combination does not interfere with any emacs shortcut, which I find suspicious :-D
Nathann
comment:10 Changed 2 years ago by dcoudert
Very nice patch. Installation, compilation and docbuild OK on sage-5.0.beta13 !
I have 2 observations:
- The width of the column should rather be 30, 70 instead of 20, 80. Of course it cannot be perfect, but depending on the size of the window of your navigator, the rendering is very different. Of course with a full screen window its perfect, but if the default size is not fullscreen, then column are not aligned between tables. It's not a big issue, but...
- Small typo in file digraph.py: Acyclicityy -> acyclicity
This should definitely be done in all files (not necessarily in this patch). For instance, I will modify patch #12716 to have the same kind of tables.
comment:11 Changed 2 years ago by ncohen
Dooooooooone !
Nathann
comment:12 Changed 2 years ago by dcoudert
This is much nicer ;-)
I did sage -t --long --force_lib sage/graphs/ and it fails (only) on generic_graph.py: TAB character found. I think we have 2 TABs in the file (functions antisymmetric and is_clique).
The patch will then be good to go.
comment:13 Changed 2 years ago by dcoudert
- Dependencies changed from 12743 to #12743
comment:14 Changed 2 years ago by ncohen
Pom Pom Pom....
Updated ! :-)
Nathann
comment:15 Changed 2 years ago by dcoudert
- Status changed from needs_review to positive_review
All tests pass and the tables are really useful. Nice work Nathann !
comment:16 Changed 2 years ago by ncohen
Wouhouuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu !!!!
Or rather, in english :
WoooooooooooHooooooooooooooooooooooo? !! :-D
Nathann
comment:17 Changed 2 years ago by jdemeyer
comment:18 Changed 2 years ago by jdemeyer
- Status changed from positive_review to needs_work
Please keep
AUTHORS:
as before, or convince people on sage-devel that
**Authors**
is better.
Also, I'm not so fond of simply removing all whitespace lines in places where there are no other changes, as this can cause merge conflicts for future patches for no good reason.
comment:19 Changed 2 years ago by ncohen
- Status changed from needs_work to positive_review
Hellooooooo Jeroen !
I just updated the patch so that the occurrences of * * Authors * * disappear. I think that it is better to write it in bold in the documentation of modules when there is actually a lot of doc as it is otherwise totally buried by the other informations (and it does not deserve to be granted a whole section).
Well, and I took care of only removing "all the trailing whitespaces" from files which I know are almost never updated. If I were to do that with graph.py or digraph.py the sage-combinat guys would kill me instantly :-D
Nathann
comment:20 Changed 2 years ago by jdemeyer
- Milestone changed from sage-5.0 to sage-5.1
comment:21 Changed 2 years ago by ncohen
(rebased on beta14)
comment:22 Changed 2 years ago by dcoudert
all tests pass ;-)
comment:23 Changed 2 years ago by jdemeyer
- Status changed from positive_review to needs_work
This doesn't apply properly on sage-5.0.beta14 + #12743:
applying trac_12816-v2.patch patching file sage/graphs/graph.py Hunk #2 FAILED at 176 1 out of 4 hunks FAILED -- saving rejects to file sage/graphs/graph.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_12816-v2.patch
comment:24 Changed 2 years ago by ncohen
- Status changed from needs_work to positive_review
Just a trailing whitespace... Updated ! :-)
Nathann
comment:25 Changed 2 years ago by jdemeyer
- Dependencies changed from #12743 to #12743, #12903
- Status changed from positive_review to needs_work
comment:26 Changed 2 years ago by ncohen
- Status changed from needs_work to positive_review
No prob ! :-)
Nathann
Changed 2 years ago by ncohen
comment:27 Changed 2 years ago by jdemeyer
- Merged in set to sage-5.1.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
The patch passes successfully a sage -t --long -force_lib --verbose sage/graphs test, and the doc builds properly. The result is really nice.
Clearly, it would be nice to have such sections on top of all pages.
For graph colorings, you have organized the methods by type. What about organizing methods by type in graph.py, generic_graph.py, etc. ?
I like this patch.