#13820: This patch implements the construction of the Johnson graph.
---------------------------------+------------------------------------------
Reporter: slani | Owner: slani
Type: task | Status: needs_work
Priority: major | Milestone: sage-5.6
Component: graph theory | Resolution:
Keywords: Johnson graph | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Uros Slana | Merged in:
Dependencies: | Stopgaps:
---------------------------------+------------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Helloooooooooooooooooo Uros !
Thank you for this addition ! Your patch looks nice indeed, but there are
a few things missing :
* We try to always write a one-line description of what the function does
on the first line of its documentation. Even if it is just, in your case
"Returns the Johnson graph of parameters n,k" `:-)`
* You can put some latex inside of the documentation of a method if you
want by using ` characters. You can look at other methods to see how it is
done.
* There is a problem with your "::". There should be only one after
"Examples", and the explanations like "The Johnson graph is a Hamiltonian
graph" should have less indentation. This is not totally useless, for the
explanation behind all lies there
{{{
sage -b && sage -docbuild reference html
}}}
Build the HTML documentation of Sage, where you can see the result of
all these indentations and "::" indications. That's the point, and usually
if it is displayed correctly then everything is good. There should not be
any warning either when you build the documentation, or the release
manager will complain !
* It would be good to define an embedding for this graph if there is a
meaningful one. Or a pretty one. You can look at other methods of the same
file to see how it is to be done. Of course, if there is no way to have a
good one, let's forget about it. It happens, after all.
* More technically : in order to build adjacencies, you take all pairs of
vertices and check whether their intersection has size `k-1`, and this is
really inefficient for most of the pairs of vertices do not intersect on
`k-1` elements. If would be more efficient to fix a set of size `k-1`, to
generate all sets of k elements containing these k-1 elements, and to add
an edge between them.
Here it issssssssss ! `:-)`
And as usual, if you think any of these remarks is not good feel free to
say it, we just try to make Sage as great as possible, and as I make
mistake when I write patches I also make mistakes when I review them `:-P`
Have fuuuuuuuuuuuuuuuuun ! And thank you again !
Nathann
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13820#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
