Opened 12 years ago

Closed 12 years ago

#1761 closed enhancement (fixed)

[with patch, positive review] Graphviz output for graphs

Reported by: boothby Owned by: rlm
Priority: minor Milestone: sage-3.0
Component: graph theory Keywords: graph visualization dot graphviz
Cc: ncalexan Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Add functionality to use graphviz for graph display.

Attachments (4)

graphviz.hg (2.2 KB) - added by boothby 12 years ago.
1761-ncalexan-graphviz-1.patch (5.9 KB) - added by ncalexan 12 years ago.
trac_1761-doctest-fix.patch (676 bytes) - added by mabshoff 12 years ago.
doctest fix due to bitrot
trac_1761-tmp-file-dir-fix.patch (1.1 KB) - added by mabshoff 12 years ago.
create doctest files in SAGE_TESTDIR

Download all attachments as: .zip

Change History (12)

Changed 12 years ago by boothby

comment:1 Changed 12 years ago by mhansen

  • Summary changed from Graphviz output for graphs to [with bundle, needs review] Graphviz output for graphs

comment:2 Changed 12 years ago by mabshoff

  • Milestone set to sage-2.10

comment:3 Changed 12 years ago by ncalexan

  • Summary changed from [with bundle, needs review] Graphviz output for graphs to [with bundle, with mostly positive review] Graphviz output for graphs

This is useful and should be applied. I would have liked to see the actual function abstracted to a Graph class higher in the hierarchy, if there is one, because it seems like the actual code is the same, just the edge identifier symbols for dot are different. Also, is '# not tested' the correct tag for doctests to not get run?

comment:4 Changed 12 years ago by jason

  • Summary changed from [with bundle, with mostly positive review] Graphviz output for graphs to [with bundle, with two mostly positive reviews, but changes recommended] Graphviz output for graphs

I agree with ncalexan:

  1. The string functions should be abstracted to the GenericGraph? class with the string changing depending on whether the graph is directed or not. In general, we are trying to consolidate functionality to the GenericGraph? class when possible these days.
  1. The graphviz function documentation should clearly state that graphviz (and in particular, dot) needs to be installed in the system path. It would be nice were the option to run the other graphviz programs, like neato, etc.

comment:5 Changed 12 years ago by ncalexan

  • Cc ncalexan added
  • Keywords graph visualization dot graphviz added
  • Summary changed from [with bundle, with two mostly positive reviews, but changes recommended] Graphviz output for graphs to [with patch, needs review] Graphviz output for graphs

The attached patch addresses the referee's comments.

It also removes references to the actual graphviz/dot application. I don't see how this can be made cross-platform and I'm worried about licensing issues, so I'm just ducking the issue. Open a new ticket if you'd like this functionality.

Changed 12 years ago by ncalexan

comment:6 Changed 12 years ago by ncalexan

The original bundle is not needed; apply only the patch.

comment:7 Changed 12 years ago by boothby

  • Summary changed from [with patch, needs review] Graphviz output for graphs to [with patch, positive review] Graphviz output for graphs

Looks good to me.

comment:8 Changed 12 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.0.alpha3

Changed 12 years ago by mabshoff

doctest fix due to bitrot

Changed 12 years ago by mabshoff

create doctest files in SAGE_TESTDIR

Note: See TracTickets for help on using tickets.