Opened 4 years ago

Closed 4 years ago

#25121 closed defect (fixed)

fix edge color format in graphviz_string

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.2
Component: graphics Keywords: thursdaysbdx
Cc: slabbe, nthiery Merged in:
Authors: Vincent Delecroix Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: e3e3ff4 (Commits, GitHub, GitLab) Commit: e3e3ff4ae8c9994d9d98ad2901f8c424a19fde78
Dependencies: #25120 Stopgaps:

Status badges

Description (last modified by slabbe)

graphviz color format should be one of the following

  • HSV triple such as ".08396, 0.4862,0.8549"
  • RGB triple such as #DA70D6
  • a color name such as orchid

As a consequence the following is broken

sage: G = Graph([(0,1)])
sage: G.set_latex_options(edge_colors={(0,1): (0.3, 0.9, 0.0)}, format='dot2tex')
sage: view(G)

To make it coherent with matplotlib option as in

sage: plot(x, color=(0.3, 0.9, 0))

we convert 3-tuples in RGB format using the to_hex function from matplotlib.

Change History (10)

comment:1 Changed 4 years ago by vdelecroix

  • Branch set to u/vdelecroix/25121
  • Commit set to 334b48ab9d07cfac3e609a1e79ef6f9683fa71b0
  • Status changed from new to needs_review

New commits:

b4176d5dirty fix for edge colors with dot2tex
334b48afix graphviz string color handling

comment:2 Changed 4 years ago by vdelecroix

  • Description modified (diff)

comment:3 Changed 4 years ago by slabbe

Why do you do this?

+to_hex = None   # overriden when needed in graphviz_string

to gain some time at start up?

sage: %time from matplotlib.colors import to_hex
CPU times: user 60 ms, sys: 12 ms, total: 72 ms
Wall time: 70.7 ms

Then why not lazy import it?

comment:4 Changed 4 years ago by git

  • Commit changed from 334b48ab9d07cfac3e609a1e79ef6f9683fa71b0 to e72b93ef6fc30f88faa1617e67024b1b8e0744b6

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

e72b93euse lazy import for to_hex

comment:5 Changed 4 years ago by vdelecroix

Good suggestion! Thanks.

comment:6 Changed 4 years ago by slabbe

  • Description modified (diff)

Adding format='dot2tex' in the description otherwise view(G) just works and does not illustrate the problem.

comment:7 Changed 4 years ago by slabbe

  • Branch changed from u/vdelecroix/25121 to u/slabbe/25121
  • Commit changed from e72b93ef6fc30f88faa1617e67024b1b8e0744b6 to e3e3ff4ae8c9994d9d98ad2901f8c424a19fde78
  • Reviewers set to Sébastien Labbé
sage -tp --optional=dot2tex,sage,graphviz --show-skipped src/sage/graphs/

looks good.

I added a very small change. If you agree, change this ticket to positive review.

New commits:

e3e3ff425121: keep_alpha=False (Explicit is better than implicit.)

comment:8 Changed 4 years ago by slabbe

  • Keywords thursdaysbdx added

comment:9 Changed 4 years ago by vdelecroix

  • Status changed from needs_review to positive_review

Merci Sébastien!

comment:10 Changed 4 years ago by vbraun

  • Branch changed from u/slabbe/25121 to e3e3ff4ae8c9994d9d98ad2901f8c424a19fde78
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.