Opened 5 years ago
Closed 5 years ago
#21048 closed enhancement (fixed)
Graph plotting: vertex colors
Reported by:  jmantysalo  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  graph theory  Keywords:  
Cc:  paulmasson, krcisman, tscrim  Merged in:  
Authors:  Jori Mäntysalo  Reviewers:  Paul Masson 
Report Upstream:  N/A  Work issues:  
Branch:  34be2bb (Commits, GitHub, GitLab)  Commit:  34be2bb7941f2e6f343b9c88ef42e02d7ba77522 
Dependencies:  Stopgaps: 
Description
This is a first part of #13827.
Currently setting a color for one vertex will change default pink to blue in other vertices. This patch will correct it. Also we make it possible to set a default color with vertex_color
and have some vertices colored differently with vertex_colors
. To test try for example
sage: g=graphs.PetersenGraph() sage: g.show() sage: g.show(vertex_color='green') sage: g.show(vertex_colors={'red': [0,1], 'blue': [3,4]}) sage: g.show(vertex_color='green', vertex_colors={'red': [0,1], 'blue': [3,4]}) sage: g.show(vertex_colors='green')
Change History (19)
comment:1 Changed 5 years ago by
 Branch set to u/jmantysalo/vertexcolor
comment:2 Changed 5 years ago by
 Cc paulmasson added
 Commit set to b8e8b5c5a12cc94128a230bc2346a6f32d49ddb0
 Status changed from new to needs_review
comment:3 followup: ↓ 4 Changed 5 years ago by
 Reviewers set to Paul Masson
Doctests pass. Documentation builds. Example runs as expected.
It looks like you took out some default rainbow coloring in the code. Since this is a part of Sage I'm unlikely to use much, I would like to understand the impact this will have on other users. While it's certainly better to have more control over the coloring, how many people are likely to complain if the rainbow coloring is gone?
comment:4 in reply to: ↑ 3 Changed 5 years ago by
Replying to paulmasson:
It looks like you took out some default rainbow coloring in the code.
I was not able to run that part of code. It seems to refer partition
option. However, currently
g = graphs.PetersenGraph() g.show(partition=[[1,2],[3,4]])
have same problem of changing default color for nonlisted vertices. With this patch they will stay pink, and also you can say
g.show(partition=[[1,2],[3,4]], vertex_color='gray')
comment:5 Changed 5 years ago by
Sidenote, there was also a plain error in documentation: "If a vertex is not listed, it looks invisible on the resulting plot (it does not get drawn)." was not true, and is not after this patch.
comment:6 followup: ↓ 7 Changed 5 years ago by
 Cc ncohen krcisman tscrim added
The current code doesn't appear to have a default color. Nonlisted vertices are given a random color, not a default color. This ticket modifies that behavior.
If that is a good idea then I'm fine with it, but I would rather have some additional input. Nathann Cohen wrote the original coloring code in 2009, so I'd like to hear from him or interested others on this change.
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Replying to paulmasson:
The current code doesn't appear to have a default color. Nonlisted vertices are given a random color, not a default color.
Kind of, but always same color. Setting a color of one vertex to blue or green make others red, setting a color to red make others light blue etc.
If that is a good idea then I'm fine with it, but I would rather have some additional input. Nathann Cohen wrote the original coloring code in 2009, so I'd like to hear from him or interested others on this change.
Me too, but Nathann faded from developers some time ago. :=(
See for example https://groups.google.com/forum/#!topic/sagedevel/Z5kXJv6ZQA
comment:8 Changed 5 years ago by
 Cc ncohen removed
Jori, I started reading sagedevel some time after that thread, so thanks for sharing. I've seen some other comments since then about the same topic but didn't realize it was such an issue. I've removed him from this ticket.
I'd like to wait a couple days to see if KarlDieter or Travis want to chime in. If we don't hear from them soon then I'll set positive review. I certainly don't want to deter an active developer!
comment:9 Changed 5 years ago by
I don't have a real preference for this because I never really use it. If you want additional input, you're probably best posting to sagedevel.
comment:10 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:11 Changed 5 years ago by
 Status changed from positive_review to needs_work
sage t long src/sage/combinat/words/finite_word.py ********************************************************************** File "src/sage/combinat/words/finite_word.py", line 116, in sage.combinat.words.finite_word Failed example: st.show(word_labels=True) Expected nothing Got: doctest:270: DeprecationWarning: Use of vertex_colors=<string> is deprecated, use vertex_color=<string> and/or vertex_colors=<dict>. See http://trac.sagemath.org/21048 for details. ********************************************************************** 1 item had failures: 1 of 56 in sage.combinat.words.finite_word [1244 tests, 1 failure, 11.63 s] sage t long src/sage/combinat/words/suffix_trees.py ********************************************************************** File "src/sage/combinat/words/suffix_trees.py", line 867, in sage.combinat.words.suffix_trees.ImplicitSuffixTree.? Failed example: ImplicitSuffixTree(Word('cacao')).plot(word_labels=True) Expected: Graphics object consisting of 23 graphics primitives Got: doctest:270: DeprecationWarning: Use of vertex_colors=<string> is deprecated, use vertex_color=<string> and/or vertex_colors=<dict>. See http://trac.sagemath.org/21048 for details. Graphics object consisting of 23 graphics primitives ********************************************************************** 1 item had failures: 1 of 7 in sage.combinat.words.suffix_trees.ImplicitSuffixTree.? [245 tests, 1 failure, 3.03 s]
comment:12 Changed 5 years ago by
 Commit changed from b8e8b5c5a12cc94128a230bc2346a6f32d49ddb0 to 60b487d3101146d4b8b10f0ae5eec193613880cc
Branch pushed to git repo; I updated commit sha1. New commits:
60b487d  Corrections to words.

comment:13 Changed 5 years ago by
 Status changed from needs_work to needs_review
Badly corrected, but at least works. I guess words and posetsdirectories should be checked when graph plotting is ready.
comment:14 Changed 5 years ago by
vertex_colors
is defined as a dicitionary in suffix_trees.py
as it should be, so the changes you made aren't the best. The problem is the misspelling veretex_colors
on line 885. Please revert the changes you just made and correct that spelling. I've already tested this and it solves both doctest failures.
comment:15 Changed 5 years ago by
 Commit changed from 60b487d3101146d4b8b10f0ae5eec193613880cc to 2e599755176bcdd903f9c2fee015df7e32345b25
Branch pushed to git repo; I updated commit sha1. New commits:
2e59975  Revert "Corrections to words."

comment:16 Changed 5 years ago by
 Commit changed from 2e599755176bcdd903f9c2fee015df7e32345b25 to 34be2bb7941f2e6f343b9c88ef42e02d7ba77522
Branch pushed to git repo; I updated commit sha1. New commits:
34be2bb  Typo on variable name corrected.

comment:17 Changed 5 years ago by
I was blind.
comment:18 Changed 5 years ago by
 Status changed from needs_review to positive_review
All doctests pass in combinat/words
and combinat/posets
comment:19 Changed 5 years ago by
 Branch changed from u/jmantysalo/vertexcolor to 34be2bb7941f2e6f343b9c88ef42e02d7ba77522
 Resolution set to fixed
 Status changed from positive_review to closed
On a way to more customizable graph plotting.
New commits:
Modified setting of graph vertices colors.