Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix edge color format in graphviz_string #25121

Closed
videlec opened this issue Apr 8, 2018 · 17 comments
Closed

fix edge color format in graphviz_string #25121

videlec opened this issue Apr 8, 2018 · 17 comments

Comments

@videlec
Copy link
Contributor

videlec commented Apr 8, 2018

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.

Depends on #25120

CC: @seblabbe @nthiery

Component: graphics

Keywords: thursdaysbdx

Author: Vincent Delecroix

Branch/Commit: e3e3ff4

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/25121

@videlec videlec added this to the sage-8.2 milestone Apr 8, 2018
@videlec
Copy link
Contributor Author

videlec commented Apr 8, 2018

New commits:

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

@videlec
Copy link
Contributor Author

videlec commented Apr 8, 2018

Commit: 334b48a

@videlec
Copy link
Contributor Author

videlec commented Apr 8, 2018

Branch: u/vdelecroix/25121

@videlec

This comment has been minimized.

@seblabbe
Copy link
Contributor

seblabbe commented Apr 9, 2018

comment:3

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?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2018

Changed commit from 334b48a to e72b93e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2018

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

e72b93euse lazy import for to_hex

@videlec
Copy link
Contributor Author

videlec commented Apr 11, 2018

comment:5

Good suggestion! Thanks.

@seblabbe
Copy link
Contributor

comment:6

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

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor

Changed branch from u/vdelecroix/25121 to u/slabbe/25121

@seblabbe
Copy link
Contributor

Changed commit from e72b93e to e3e3ff4

@seblabbe
Copy link
Contributor

comment:7
sage -tp --optional=dot2tex,sage,graphviz --show-skipped src/sage/graphs/generic_graph.py

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.)

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@seblabbe
Copy link
Contributor

Changed keywords from none to thursdaysbdx

@videlec
Copy link
Contributor Author

videlec commented Apr 12, 2018

comment:9

Merci Sébastien!

@vbraun
Copy link
Member

vbraun commented May 12, 2018

Changed branch from u/slabbe/25121 to e3e3ff4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants