Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#26469 closed enhancement (fixed)

avoid sorting vertex labels in graph_plot.py

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: tscrim, chapoton, tmonteil, dimpase Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 02074cd (Commits) Commit: 02074cd252f67668a7789bc980c0421e6768b476
Dependencies: Stopgaps:

Description (last modified by dcoudert)

We avoid sorting vertex labels in method set_edges of graph_plot.py.

Change History (11)

comment:1 Changed 6 months ago by dcoudert

  • Branch set to public/26469_graph_plot_py
  • Cc tscrim chapoton tmonteil dimpase added
  • Commit set to c713cb9b6b1691c6ab45ad5462756ce3eecaf8fc
  • Description modified (diff)
  • Status changed from new to needs_review

I already did several tests, but more tests are certainly needed.

I set this ticket to needs review, but before finalizing it, I plan to clean the entire file (PEP8). So let me know if you agree or not with proposed change.

comment:2 Changed 6 months ago by git

  • Commit changed from c713cb9b6b1691c6ab45ad5462756ce3eecaf8fc to 9d604a84699da569b6f7a652d8299c757e741535

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

9d604a8trac #26469: pyflakes

comment:3 Changed 5 months ago by tscrim

Yep, this is fine with me (I'm not worried about speed in plotting, but for other tickets, I would not want to move the is_directed() calls into the for loop).

comment:4 Changed 5 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

comment:5 Changed 5 months ago by git

  • Commit changed from 9d604a84699da569b6f7a652d8299c757e741535 to 3d8d7d99f68af4a35d4b39024cb03e1caf08dcc8

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

3d8d7d9trac #26469: some cleaning

comment:6 Changed 5 months ago by dcoudert

I cleaned the file (and moved the is_directed outside the for loop).

comment:7 Changed 5 months ago by tscrim

While we are bring things up to standard:

Returns -> Return and Sets -> Set.

         - ``orientation`` -- whether to place the root at the top or at the
-          bottom :
+          bottom:
 
-            - ``orientation="down"`` -- children are placed below their parent
-            - ``orientation="top"`` -- children are placed above their parent
+          * ``orientation="down"`` -- children are placed below their parent
+          * ``orientation="top"`` -- children are placed above their parent

Everything else is okay.

comment:8 Changed 5 months ago by git

  • Commit changed from 3d8d7d99f68af4a35d4b39024cb03e1caf08dcc8 to 02074cd252f67668a7789bc980c0421e6768b476

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

02074cdtrac #26469: reviewers comments

comment:9 Changed 5 months ago by tscrim

  • Status changed from needs_review to positive_review

Thank you.

comment:10 Changed 5 months ago by vbraun

  • Branch changed from public/26469_graph_plot_py to 02074cd252f67668a7789bc980c0421e6768b476
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 Changed 5 months ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

This should be re-targeted for 8.5.

Note: See TracTickets for help on using tickets.