Opened 5 months ago

Closed 4 months ago

#26680 closed enhancement (fixed)

clean generic_graph.py (part 14) - visualization

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: tscrim, chapoton Merged in:
Authors: David Coudert Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: e2cd65a (Commits) Commit: e2cd65af17d5ae208d6e4c4cc3736ba5ea45036f
Dependencies: Stopgaps:

Description

We clean (PEP8 and avoid .vertices / .edges) methods related to graph visualization: _color_by_label, latex_options, set_latex_options, layout, layout_spring, layout_ranked, layout_extend_randomly, layout_circular, layout_tree, layout_graphviz, _layout_bounding_box, _circle_embedding, _line_embedding, graphplot, _rich_repr_, plot, show, plot3d, show3d, _keys_for_vertices, graphviz_string, graphviz_to_file_named, and tachyon_vertex_plot.

Remaining calls to .vertices:

  • layout_circular calls _circle_embedding with a list of vertices ordered by .vertices. Changing this will change the layout.
  • _keys_for_vertices returns a mapping vertex to unique string identifier as required by dot2tex. It uses the order of self.vertices() to build the mapping. It is then used in graphviz_string and layout_graphviz.

Any advice for these 2 cases is more than welcome ;)

Change History (10)

comment:1 Changed 5 months ago by dcoudert

  • Branch set to public/26680_generic_graph_part_14
  • Commit set to 5d971aa3924e2e67670339eb9099092737e8a365
  • Status changed from new to needs_review

New commits:

5d971aatrac #26680: part 14 - visualization

comment:2 Changed 5 months ago by chapoton

one failing doctest, and one plugin warning (missing r''')

comment:3 Changed 5 months ago by git

  • Commit changed from 5d971aa3924e2e67670339eb9099092737e8a365 to 5f72b6e91e5e6ca749b79dbb3ef7252d672158a4

Branch pushed to git repo; I updated commit sha1. New commits:

04c09d8trac #26680: Merged with 8.5.beta6
5f72b6etrac #26680: fix plugin warning

comment:4 Changed 5 months ago by dcoudert

I fixed the plugin warning, but I don't find the failing doctest. Which one is it ?

comment:5 Changed 5 months ago by chapoton

in the patchbot:

sage -t --long --warn-long 55.5 src/sage/graphs/generic_graph.py
**********************************************************************
File "src/sage/graphs/generic_graph.py", line 19671, in sage.graphs.generic_graph.GenericGraph.?
Failed example:
    P.plot3d(engine='tachyon', edge_colors=edge_colors).show() # long time
Exception raised:
    Traceback (most recent call last):
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.graphs.generic_graph.GenericGraph.?[20]>", line 1, in <module>
        P.plot3d(engine='tachyon', edge_colors=edge_colors).show() # long time
      File "/home/chapoton/sage/local/lib/python2.7/site-packages/sage/graphs/generic_graph.py", line 19788, in plot3d
        for u,v,l in edge_colors[color]:
    TypeError: 'int' object is not iterable

comment:6 Changed 5 months ago by git

  • Commit changed from 5f72b6e91e5e6ca749b79dbb3ef7252d672158a4 to e2cd65af17d5ae208d6e4c4cc3736ba5ea45036f

Branch pushed to git repo; I updated commit sha1. New commits:

e2cd65atrac #26680: fix doctest in plot3d

comment:7 Changed 5 months ago by dcoudert

ok, fixed.

comment:8 Changed 5 months ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, let it be. But..

  • not a good idea (rather a very bad idea) to mix many trivial pep8 changes with non trivial code changes. A few pep8 changes are ok. I myself try hard to refrain to do that, as tempting as it would be. Let us hope that your graph tickets will be reviewed nevertheless.
  • pep8 should not be enforced in the doctests, no way. It's only for the code.

comment:9 Changed 5 months ago by dcoudert

May be I should split the other tickets into easier to review tickets... Some of them are way too big.

comment:10 Changed 4 months ago by vbraun

  • Branch changed from public/26680_generic_graph_part_14 to e2cd65af17d5ae208d6e4c4cc3736ba5ea45036f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.