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

Improve ActionDigraph graphviz colour capability #125

Merged

Conversation

MTWhyte
Copy link
Contributor

@MTWhyte MTWhyte commented Mar 5, 2023

This PR improves the graphviz colour capability for ActionDigraph objects, as discussed in issue #100.

More specifically: a nice palate of nine colourblind accessible colours due to Paul Tol is used. Where a graph has more than nine distinct labels whose edges need coloured, these nine colours are simply reused cyclically. This seemed like a reasonable way of doing things.

@MTWhyte
Copy link
Contributor Author

MTWhyte commented Mar 5, 2023

Note that test I have is currently failing, or at least it was on my computer before I pushed. This is due to new lines in the string corresponding to the dot object. I've tried to do this in a similar way to the previous test, but it is not working (possibly this is because the string is quite long?), and I don't know how to get it to behave.

d = action_digraph_helper.make(4, l)
assert (
str(action_digraph_helper.dot(d))
== 'digraph {\n\tnode [shape=circle]\n\t0\n\t1\n\t2\n\t3\n\t0 -> 0 [label=0 color="#cc6677"]\n\t0 -> 0 [label=1 color="#ddcc77"]\n\t0 -> 0 [label=2 color="#117733"]\n\t0 -> 0 [label=3 color="#88ccee"]\n\t0 -> 0 [label=4 color="#44aa99"]\n\t0 -> 0 [label=5 color="#882255"]\n\t0 -> 0 [label=6 color="#44aa99"]\n\t0 -> 0 [label=7 color="#999933"]\n\t0 -> 0 [label=8 color="#332288"]\n\t0 -> 0 [label=9 color="#cc6677"]\n\t0 -> 0 [label=10 color="#ddcc77"]\n\t0 -> 0 [label=11 color="#117733"]\n\t1 -> 1 [label=0 color="#cc6677"]\n\t1 -> 1 [label=1 color="#ddcc77"]\n\t1 -> 1 [label=2 color="#117733"]\n\t1 -> 1 [label=3 color="#88ccee"]\n\t1 -> 1 [label=4 color="#44aa99"]\n\t1 -> 1 [label=5 color="#882255"]\n\t1 -> 1 [label=6 color="#44aa99"]\n\t1 -> 1 [label=7 color="#999933"]\n\t1 -> 1 [label=8 color="#332288"]\n\t1 -> 1 [label=9 color="#cc6677"]\n\t1 -> 1 [label=10 color="#ddcc77"]\n\t1 -> 1 [label=11 color="#117733"]\n\t2 -> 2 [label=0 color="#cc6677"]\n\t2 -> 2 [label=1 color="#ddcc77"]\n\t2 -> 2 [label=2 color="#117733"]\n\t2 -> 2 [label=3 color="#88ccee"]\n\t2 -> 2 [label=4 color="#44aa99"]\n\t2 -> 2 [label=5 color="#882255"]\n\t2 -> 2 [label=6 color="#44aa99"]\n\t2 -> 2 [label=7 color="#999933"]\n\t2 -> 2 [label=8 color="#332288"]\n\t2 -> 2 [label=9 color="#cc6677"]\n\t2 -> 2 [label=10 color="#ddcc77"]\n\t2 -> 2 [label=11 color="#117733"]\n\t3 -> 3 [label=0 color="#cc6677"]\n\t3 -> 3 [label=1 color="#ddcc77"]\n\t3 -> 3 [label=2 color="#117733"]\n\t3 -> 3 [label=3 color="#88ccee"]\n\t3 -> 3 [label=4 color="#44aa99"]\n\t3 -> 3 [label=5 color="#882255"]\n\t3 -> 3 [label=6 color="#44aa99"]\n\t3 -> 3 [label=7 color="#999933"]\n\t3 -> 3 [label=8 color="#332288"]\n\t3 -> 3 [label=9 color="#cc6677"]\n\t3 -> 3 [label=10 color="#ddcc77"]\n\t3 -> 3 [label=11 color="#117733"]\n}\n' # pylint: disable=line-too-long
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to escape the quotes in, for example using color=\"#44aa99\" otherwise you are just ending the string, if you see what I mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, is this definitely the problem? Even though they don't pass, I the test still does run. I think since the outer quotes are different (' vs "), that Python can parse this ok.

For example in iPython I get

In [1]: print('1 + 2 is "3"')
1 + 2 is "3"

Possibly it behaves differently in the tests, but if I add a single quote rather than a double at a random place in the string, that does break the syntax and the test doesn't even run.

@MTWhyte
Copy link
Contributor Author

MTWhyte commented Mar 6, 2023

Turns out I misread which test was failing (in short: with my new setup I thought I was just deleting, but it was in fact cutting). Fixed it now!

I was having some trouble shortening the lines and using + since the formatter wouldn't be consistent about which quotes to use, so I've just left that as is for the time being.

@james-d-mitchell
Copy link
Member

Looks good to me, ok to merge @MTWhyte ?

@MTWhyte
Copy link
Contributor Author

MTWhyte commented Mar 6, 2023

Yeah go ahead!

@james-d-mitchell james-d-mitchell merged commit 9cebb3a into libsemigroups:main Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants