Opened 6 years ago
Closed 6 years ago
#10723 closed enhancement (fixed)
Implement edge-by-edge options for latexing graphs with dot2tex
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | graph theory | Keywords: | graph, latex, dot2tex |
Cc: | sage-combinat, rbeezer | Merged in: | sage-4.7.alpha2 |
Authors: | Nicolas M. Thiéry | Reviewers: | Rob Beezer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Implement edge-by-edge options for latexing graphs with dot2tex
Here we color in red all edges touching the vertex 0
::
sage: G = graphs.PetersenGraph() sage: G.set_latex_options(format="dot2tex", edge_options = lambda (u,v,label): { "color": "red" if u==0 else 1}) sage: view(G)
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
Attachments (1)
Change History (20)
comment:1 Changed 6 years ago by
- Description modified (diff)
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:4 Changed 6 years ago by
- Description modified (diff)
- Summary changed from Implement edge-by-edge option for ploting/viewing/latexing graphs to Implement edge-by-edge options for latexing graphs with dot2tex
comment:5 Changed 6 years ago by
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 Changed 6 years ago by
- Owner changed from jason, ncohen, rlm to nthiery
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:7 Changed 6 years ago by
- Cc rbeezer added
comment:8 Changed 6 years ago by
Comment from Dan Bump:
Currently after #10723 if dot2tex is not installed there are some doc failures in:
graph_plot.py generic_graph.py
comment:9 Changed 6 years ago by
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 follow-up: ↓ 11 Changed 6 years ago by
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 x
*from* quotes and newlines.
Not part of the patch, but while you are here... Native speaker advice: I'd say "strip string *of* quotes"
generic_graph.py:
line 12199:
The doctest with the Cayley graph is not very instructive if you do not know what the (automatically-supplied) labels are - especially if you don't know any group theory. Maybe just a line or two describing the graph's labels before you go to work with them?
line 12212:
I did not understand the warning until I re-read one of your emails. "Collision" suggests to me that one edge is getting two colors (two colors in the same place). Instead, I think you mean that the "rainbow" assignments as a default will provide a color to some other edge, which is identical to a color that is user-supplied for an edge. Is that right? I think this is a bit "dangerous" - a user could wonder how "their" color ended up on other edges?
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
color_by_label
should beedge_color_by_label
. One might want to do exactly the same thing with the vertices in the future? Or it should be unambiguous just what is being colored?
line 857:
edge_labels
(plural) is a good change.
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
comment:11 in reply to: ↑ 10 Changed 6 years ago by
- Reviewers set to Rob Beezer
Hi Robert!
Thanks for your review.
Replying to rbeezer:
dot2tex_utils.py, line 95: strips string representation of
x
*from* quotes and newlines.
Not part of the patch, but while you are here... Native speaker advice: I'd say "strip string *of* quotes"
Done.
generic_graph.py:
line 12199:
The doctest with the Cayley graph is not very instructive if you do not know what the (automatically-supplied) labels are - especially if you don't know any group theory. Maybe just a line or two describing the graph's labels before you go to work with them?
Done.
line 12212:
I did not understand the warning until I re-read one of your emails. "Collision" suggests to me that one edge is getting two colors (two colors in the same place). Instead, I think you mean that the "rainbow" assignments as a default will provide a color to some other edge, which is identical to a color that is user-supplied for an edge. Is that right? I think this is a bit "dangerous" - a user could wonder how "their" color ended up on other edges?
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).
Agreed. It now uses the default color.
graph_latex.py:
line 226: What happens when you try to view(G)? (Answer: I think this is one of the doctest failures.)
Replaced by a call to latex as above in the same file.
line 534: I really think
color_by_label
should beedge_color_by_label
. One might want to do exactly the same thing with the vertices in the future? Or it should be unambiguous just what is being colored?
I agree, but this option already exists with that name in plot/plot3d. So if we decide for this change then this should be done everywhere at once. I vote for postponing that to #10851.
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
All test pass on my machine with the updated patch I'll upload soon. If I did not screw up, this is both with and without dot2tex, as well as with -long and -optional.
Cheers,
Nicolas
Changed 6 years ago by
comment:12 Changed 6 years ago by
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 follow-up: ↓ 14 Changed 6 years ago by
- Status changed from needs_review to needs_info
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:
make[3]: *** No rule to make target `../../plugin/pango/libgvplugin_pango.la', needed by `dot_builtins'. Stop. make[3]: *** Waiting for unfinished jobs.... mv -f .deps/dot_builtins.Tpo .deps/dot_builtins.Po mv -f .deps/no_demand_loading.Tpo .deps/no_demand_loading.Po mv -f .deps/dot.Tpo .deps/dot.Po make[3]: Leaving directory `/sage/dev/spkg/build/graphviz-2.16.1.p0/src/cmd/dot' make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory `/sage/dev/spkg/build/graphviz-2.16.1.p0/src/cmd' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/sage/dev/spkg/build/graphviz-2.16.1.p0/src' make: *** [all] Error 2 Error building Graphviz
In this state, I get just a few failures using
sage -tp 9 -only-optional "dot2tex" sage/graphs
that start with errors below. What am I doing wrong?
Rob
sage -t -only-optional sage/graphs/generic_graph.py ERROR Failed to process input Traceback (most recent call last): File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 2928, in main s = conv.convert(dotdata) File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 848, in convert return self.output() File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 2566, in output positions[node.name] = map(int, pos.split(',')) ValueError: invalid literal for int() with base 10: '87.137' ERROR Failed to process input Traceback (most recent call last): File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 2928, in main s = conv.convert(dotdata) File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 848, in convert return self.output() File "/sage/dev/local/lib/python/site-packages/dot2tex/dot2tex.py", line 2566, in output positions[node.name] = map(int, pos.split(',')) ValueError: invalid literal for int() with base 10: '239.56' ********************************************************************** File "/sage/dev/devel/sage-main/sage/graphs/generic_graph.py", line 12702: sage: g.plot(layout = "graphviz", prog = "neato") # optional - dot2tex, graphviz Exception raised: Traceback (most recent call last): File "/sage/dev/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/sage/dev/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/sage/dev/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_0[6]>", line 1, in <module> g.plot(layout = "graphviz", prog = "neato") # optional - dot2tex, graphviz###line 12702: sage: g.plot(layout = "graphviz", prog = "neato") # optional - dot2tex, graphviz File "/sage/dev/local/lib/python/site-packages/sage/misc/decorators.py", line 432, in wrapper return func(*args, **options) File "/sage/dev/local/lib/python/site-packages/sage/graphs/generic_graph.py", line 13078, in plot return GraphPlot(graph=self, options=options).plot() File "/sage/dev/local/lib/python/site-packages/sage/graphs/graph_plot.py", line 95, in __init__ self.set_pos() File "/sage/dev/local/lib/python/site-packages/sage/graphs/graph_plot.py", line 141, in set_pos self._pos = self._graph.layout(**self._options) File "/sage/dev/local/lib/python/site-packages/sage/graphs/generic_graph.py", line 12425, in layout pos = getattr(self, "layout_%s"%layout)(dim = dim, **options) File "/sage/dev/local/lib/python/site-packages/sage/graphs/generic_graph.py", line 12732, in layout_graphviz return dict( (key_to_vertex[key], pos) for (key, pos) in positions.iteritems() ) AttributeError: 'str' object has no attribute 'iteritems'
comment:14 in reply to: ↑ 13 Changed 6 years ago by
- Status changed from needs_info to needs_review
Replying to rbeezer:
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:
Please create a ticket about the Sage graphviz package (if it does not already exists). Honestly, I always use my system-wide install :-)
In this state, I get just a few failures using
sage -tp 9 -only-optional "dot2tex" sage/graphsthat start with errors below. What am I doing wrong?
That's exactly what I meant with the above comment:
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.
Cheers,
Nicolas
comment:15 Changed 6 years ago by
comment:16 follow-up: ↓ 18 Changed 6 years ago by
- Status changed from needs_review to positive_review
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 Changed 6 years ago by
comment:18 in reply to: ↑ 16 Changed 6 years ago by
Hi Rob!
Replying to rbeezer:
Sorry for the delay on this one. I didn't realize it was holding up some other tickets.
I should have mentioned it in the first place :-)
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.
Thanks much for the review!
And yes, we should try to coordinate sometime on the different approaches to making these pretty pictures.
And even prettier :-)
Cheers,
Nicolas
comment:19 Changed 6 years ago by
- Merged in set to sage-4.7.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
(cc me)