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)
Change History (12)
Changed 12 years ago by
comment:1 Changed 12 years ago by
- Summary changed from Graphviz output for graphs to [with bundle, needs review] Graphviz output for graphs
comment:2 Changed 12 years ago by
- Milestone set to sage-2.10
comment:3 Changed 12 years ago by
- Summary changed from [with bundle, needs review] Graphviz output for graphs to [with bundle, with mostly positive review] Graphviz output for graphs
comment:4 Changed 12 years ago by
- 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:
- 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.
- 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
- 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
comment:6 Changed 12 years ago by
The original bundle is not needed; apply only the patch.
comment:7 Changed 12 years ago by
- 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
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 3.0.alpha3
Note: See
TracTickets for help on using
tickets.
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?