Opened 7 years ago

Closed 7 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 nthiery)

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)

trac_10723-graph_edge_options-nt.patch (31.8 KB) - added by nthiery 7 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 7 years ago by stumpc5

  • Description modified (diff)

comment:2 Changed 7 years ago by ncohen

(cc me)

comment:3 Changed 7 years ago by nthiery

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 7 years ago by nthiery

  • 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 7 years ago by nthiery

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 7 years ago by nthiery

  • 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 7 years ago by nthiery

  • Cc rbeezer added

comment:8 Changed 7 years ago by aschilling

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 7 years ago by aschilling

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: Changed 7 years ago by rbeezer

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 be edge_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 7 years ago by nthiery

  • 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 be edge_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 7 years ago by nthiery

comment:12 Changed 7 years ago by nthiery

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: Changed 7 years ago by rbeezer

  • 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 7 years ago by nthiery

  • 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/graphs

that 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 7 years ago by nthiery

Hi Robert!

Would you have a chance to finalize the review soon?

#10632 and #10485 basically just depend on this.

Thanks!

Nicolas

comment:16 follow-up: Changed 7 years ago by rbeezer

  • 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 7 years ago by rbeezer

Nicolas,

graphviz failure is known, old, and both of us are on the ticket. ;-)

#7438

Rob

comment:18 in reply to: ↑ 16 Changed 7 years ago by nthiery

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 7 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.