-
-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement edge-by-edge options for latexing graphs with dot2tex #10723
Comments
This comment has been minimized.
This comment has been minimized.
comment:2
(cc me) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:5
For the record, until the testbot does its job: this patch passes all tests on massena (with a couple patches applied before in the Sage-Combinat queue, but they should be unrelated). |
comment:6
Note: the patch does not apply on 4.6.1, hence the current failure of the testbot. Oh, and I should mention: when I said "all test pass" above, that's to the exception of a couple timeouts in completely unrelated tests files. |
comment:8
Comment from Dan Bump: Currently after #10723 if dot2tex is not installed there are some graph_plot.py |
comment:9
It looks good to me from the user interface point of view: I especially like the feature that one can now use for a digraph G sage: G.set_latex_options(color_by_label = True) and Sage sets its own color scheme (or one can set less colors than edge labels and Sage chooses the rest). |
comment:10
Some comments and suggestions from a first pass through the patch, experimenting with the code and running doctests. dot2tex_utils.py: line 95: strips string representation of generic_graph.py: line 12199: line 12212: In any event, I really like the rainbow colors for all of the edges as a default, but the minute there are some user-supplied colors, then I think the other edges should get a single default color (probably black). graph_latex.py: line 226: What happens when you try to view(G)? (Answer: I think this is one of the doctest failures.) line 534: I really think line 857: Doctests: 4.6.2.alpha4, dot2tex-2.8.7-2, graphviz 2.26.3 sage -t devel/sage/sage/graphs/graph_plot.py # 12 doctests failed sage -t devel/sage/sage/graphs/graph_latex.py # 1 doctests failed sage -t devel/sage/sage/graphs/generic_graph.py # 4 doctests failed |
Reviewer: Rob Beezer |
comment:11
Hi Robert! Thanks for your review. Replying to @rbeezer:
Done.
Done.
Agreed. It now uses the default color.
Replaced by a call to latex as above in the same file.
I agree, but this option already exists with that name in
All test pass on my machine with the updated patch I'll upload soon. Cheers, |
Attachment: trac_10723-graph_edge_options-nt.patch.gz |
comment:12
Oops, some long tests were still failing. Fixed and uploaded updated patch. Part of the issue was due to the conflicting usage for edge_colors, where the specs said it was a dictionary, but it was used as a string in a couple places. I forked a separate option edge_color. Some optional tests are failing due to a glitch in dot2tex which I have reported upstream (asking for a dot2tex layout with neato is currently broken). In any cases, that's orthogonal to this patch. |
comment:13
I have the dot2tex-2.8.7-2 spkg installed. I have a system-wide graphviz installed, but the Sage graphviz package won't build:
In this state, I get just a few failures using
that start with errors below. What am I doing wrong? Rob
|
comment:14
Replying to @rbeezer:
Please create a ticket about the Sage graphviz package (if it does not already exists). Honestly, I always use my system-wide install :-)
That's exactly what I meant with the above comment: Cheers, |
comment:16
Hi Nicolas, Sorry for the delay on this one. I didn't realize it was holding up some other tickets. Thanks for the changes. I let the failure of graphviz to install throw me off and I forgot your warning on the optional test failures. I'll pursue reporting the problem with the graphviz package. This passes all non-optional tests, with or without, dot2tex installed, documentation builds fine, and works as advertised. So positive review. And yes, we should try to coordinate sometime on the different approaches to making these pretty pictures. Rob |
comment:17
Nicolas, graphviz failure is known, old, and both of us are on the ticket. ;-) Rob |
comment:18
Hi Rob! Replying to @rbeezer:
I should have mentioned it in the first place :-)
Thanks much for the review!
And even prettier :-) Cheers, |
Merged: sage-4.7.alpha2 |
Implement edge-by-edge options for latexing graphs with dot2tex
Here we color in red all edges touching the vertex
0
::By the way, this refactors the code of color_by_label to simplify its logic and generalize it for dot2tex output.
It also fixes a couple typos in the documentation, as well as
little shortcomings in the quotation functions in dot2tex_utils.
Patch also available from the Sage-Combinat queue:
http://combinat.sagemath.org/patches/file/tip/trac_10723-graph_edge_options-nt.patch
CC: @sagetrac-sage-combinat @rbeezer
Component: graph theory
Keywords: latex, dot2tex
Author: Nicolas M. Thiéry
Reviewer: Rob Beezer
Merged: sage-4.7.alpha2
Issue created by migration from https://trac.sagemath.org/ticket/10723
The text was updated successfully, but these errors were encountered: