#14980: graph_generators, some more clean up
--------------------------------+---------------------------
Reporter: eisermbi | Owner:
Type: enhancement | Status: needs_info
Priority: major | Milestone: sage-5.12
Component: graph theory | Resolution:
Keywords: | Merged in:
Authors: | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Dependencies:
Stopgaps: |
--------------------------------+---------------------------
Comment (by eisermbi):
Thanks for your comment!
Replying to [comment:2 ncohen]:
> [...]
>
> '''Part 1:'''
> It all makes sense to me. Individual graphs is a nice name too, btw.
This being said, do these names really appear in Sage ? That's the
module's name of course but users get those graphs through `graphs.<tab>`,
so it's not that bad either.
Since a good name is not so easy to find, let us stick with the name and
just add the description. That makes it clear enough.
> '''Part 2:'''
> - Why did you remove the index in chessboard.py ? An index is
niiiiiiiiiice `:-P`
> Plus this module will not change much, so it's not very hard to keep
updated.
Sure, I did not want to remove any index. But I am curious which index you
are refering to. I thought
{{{
__append_to_doc(["BishopGraph", "KingGraph", "KnightGraph",
"QueenGraph", "RookGraph"])
}}}
in the file "graph_generators.py" does this job nowadays. Is there another
index??
> - You changed the order of methods in the Platonic Solids, but the
`append_to_doc` method reorders them according to the alphabetical order
`:-P`
Hmmm... Looking at the definition {{{def __append_to_doc(methods):}}} I
cannot see that {{{methods}}} is reordered alphabetically nor is the
output (HTML documentation) sorted alphabetically. I get
{{{
TetrahedralGraph HexahedralGraph DodecahedralGraph
OctahedralGraph IcosahedralGraph
}}}
which is ordered by the number of vertices (not alphabetically). I think
it is working...
> - What do you mean by "chessboard graphs are now a subsection of
families" ?
In the documentation I changed the level of the heading of chessboard
graphs. Now it looks like "Chessboard graphs" is a subsection of "Families
of graphs". Similar with "Platonic solids".
> - Bad formatting of Random GNP : could you open a ticket for that and
add me in cc ?
Done, #15009.
> '''Part 3:'''
> - Why on earth wouldn't `fullerenes`, `trees`,
`line_graph_forbidden_subgraphs` and `cospectral_graphs` be families of
graphs ? `O_O`
Yes, they are families of graphs. What I wanted to say is that they do not
return a single graph but a list of graphs or graph iterators. Especially,
the function `cospectral_graphs` made me wonder since it differs from the
others in the return type - a list of lists of graphs. Is a "family"
(list) the same as "family of sets" (list of lists)? `;-)` Actually these
function can be distinguished from the others because their names start
with small letters. Hence, leaving them under "Families" is okay.
> - If these are not moves to `miscellaneous.py`, this module doesn't have
any meaning if it just contains the world map. Actually, I created an
independent file for this graph because it takes sooooooooooo much Python
code to build.
I see. I think it is easier to lookup a file when the section title is
similar to the filename, e.g. "Random graphs" <-> "random.py",
"Miscellaneous" <-> "miscellaneous.py". And the file would also be open
for new graphs other than world maps. That is my reasoning.
> '''Part 4:'''
> Wow. This patch is a bit hard to read.
>
> First, it seems from your message that after this patch is applied some
method may be defined in two files, and we just don't do that. We'd be
sure to fix a bug in one without fixing the other. Unless you imported one
into the other file, in which case my reason for opposing this change is
different :
No, I took each function and put it either in `families.py` or `small.py`
(never both!). My statement about "double listing" graphs might have been
confusing. I only was referring to the documentation not the source code.
I moved the source code of graphs considered as basic into their
corresponding sections and listed the graph names only a second time under
"Basic structures" in the reference documentation. That is all.
> First, I don't get your main argument : how can moving methods around in
these files change anything for the users, when he can get them all
through `graphs.<something>` ? These constructors have only been split
because Jeroen didn't want the `graph_generators` file to grow too large,
and the users do not even have to know that they exist. Is it just because
of the index in `graph_generators.py` ?
Partly, yes. At the moment to check whether a graph exists one also needs
to look at "Basic structures" because a graph might be listed there
instead of its corresponding section. Not so bad either. So I can drop
this part if the change is too complicated... Let me know.
> Overall:
> - You fixed typoes in `sage.graphs.graph_generators` in some places, but
could you change it to {{{:mod:`sage.graphs.graph_generators`}}} instead ?
Sure, I can.
> So far I agree with the first 3 parts, short of this question of things
which you don't think are graph families. Tell me what you think ! `;-)`
>
> Nathann
After the remaining points are clarified I will update the patch. `:-)`
Birk
--
Ticket URL: <http://trac.sagemath.org/ticket/14980#comment:3>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
