#11908: Fix tree plotting again
-------------------------------------+--------------------------------------
Reporter: boothby | Owner: jason, ncohen, rlm
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.10
Component: graph theory | Resolution:
Keywords: tree, plot | Work issues:
Report Upstream: N/A | Reviewers: Nathann Cohen
Authors: Frédéric Chapoton | Merged in:
Dependencies: | Stopgaps:
-------------------------------------+--------------------------------------
Changes (by ncohen):
* reviewer: => Nathann Cohen
Old description:
> I wrote some code that produces fairly nice-looking tree plots in #6747,
> and, I might point out, worked great. For some reason, this was
> obliterated in #7004 with code that almost does the right thing, but
> fails to draw the tree crossing-free with high probability.
>
> The easy thing is to add a parameter {{{shuffle}}} to
> {{{GenericGraph.layout_ranked}}}, so {{{GenericGraph.layout_tree}}} could
> just call {{{return self.layout_ranked(heights_dict,shuffle=False)}}} to
> avoid the (somewhat baffling) behavior of shuffling the vertices along
> the heights. IMHO, the default should be to not shuffle... but the
> primary focus of this ticket is to fix the damned tree plots. Again.
>
> My preference is to bring back my code from #6747, since the results are
> pretty, and I find the "shrink-wrapped" look of layout_ranked to be
> incomprehensible for large-ish trees. A good compromise is to add
> options: layout = "tree", "tree:hang", "tree:shrinkwrap", "tree:circle",
> etc.
>
> Note to reviewers: look at the plots produced in the doctests before
> giving a positive review.
New description:
I wrote some code that produces fairly nice-looking tree plots in #6747,
and, I might point out, worked great. For some reason, this was
obliterated in #7004 with code that almost does the right thing, but fails
to draw the tree crossing-free with high probability.
The easy thing is to add a parameter {{{shuffle}}} to
{{{GenericGraph.layout_ranked}}}, so {{{GenericGraph.layout_tree}}} could
just call {{{return self.layout_ranked(heights_dict,shuffle=False)}}} to
avoid the (somewhat baffling) behavior of shuffling the vertices along the
heights. IMHO, the default should be to not shuffle... but the primary
focus of this ticket is to fix the damned tree plots. Again.
My preference is to bring back my code from #6747, since the results are
pretty, and I find the "shrink-wrapped" look of layout_ranked to be
incomprehensible for large-ish trees. A good compromise is to add
options: layout = "tree", "tree:hang", "tree:shrinkwrap", "tree:circle",
etc.
Note to reviewers: look at the plots produced in the doctests before
giving a positive review.
Apply:
* [attachment:trac_11908-fc.patch]
* [attachment:trac_11908-rev.patch]
--
Comment:
Hellooooooooooooo !
It took me some time to understand it, but it's done. I added comments, to
help the next one who will have to read that.
If you agree with those changes, you can set the ticket to
`positive_review` ! Nice layout `:-)`
Nathann
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11908#comment:13>
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
