Opened 6 years ago

Closed 6 years ago

#21048 closed enhancement (fixed)

Graph plotting: vertex colors

Reported by: Jori Mäntysalo Owned by:
Priority: major Milestone: sage-7.3
Component: graph theory Keywords:
Cc: Paul Masson, krcisman, Travis Scrimshaw Merged in:
Authors: Jori Mäntysalo Reviewers: Paul Masson
Report Upstream: N/A Work issues:
Branch: 34be2bb (Commits, GitHub, GitLab) Commit: 34be2bb7941f2e6f343b9c88ef42e02d7ba77522
Dependencies: Stopgaps:

Status badges

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 6 years ago by Jori Mäntysalo

Branch: u/jmantysalo/vertex-color

comment:2 Changed 6 years ago by Jori Mäntysalo

Cc: Paul Masson added
Commit: b8e8b5c5a12cc94128a230bc2346a6f32d49ddb0
Status: newneeds_review

On a way to more customizable graph plotting.


New commits:

b8e8b5cModified setting of graph vertices colors.

comment:3 Changed 6 years ago by Paul Masson

Reviewers: 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?

Last edited 6 years ago by Paul Masson (previous) (diff)

comment:4 in reply to:  3 Changed 6 years ago by Jori Mäntysalo

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 non-listed 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 6 years ago by Jori Mäntysalo

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 Changed 6 years ago by Paul Masson

Cc: Nathann Cohen krcisman Travis Scrimshaw 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 6 years ago by Jori Mäntysalo

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/sage-devel/-Z5kXJv6ZQA

comment:8 Changed 6 years ago by Paul Masson

Cc: Nathann Cohen removed

Jori, I started reading sage-devel 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 Karl-Dieter 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 6 years ago by Travis Scrimshaw

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 sage-devel.

comment:10 Changed 6 years ago by Paul Masson

Status: needs_reviewpositive_review

comment:11 Changed 6 years ago by Volker Braun

Status: positive_reviewneeds_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 6 years ago by git

Commit: b8e8b5c5a12cc94128a230bc2346a6f32d49ddb060b487d3101146d4b8b10f0ae5eec193613880cc

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

60b487dCorrections to words.

comment:13 Changed 6 years ago by Jori Mäntysalo

Status: needs_workneeds_review

Badly corrected, but at least works. I guess words- and posets-directories should be checked when graph plotting is ready.

comment:14 Changed 6 years ago by Paul Masson

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.

Last edited 6 years ago by Paul Masson (previous) (diff)

comment:15 Changed 6 years ago by git

Commit: 60b487d3101146d4b8b10f0ae5eec193613880cc2e599755176bcdd903f9c2fee015df7e32345b25

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

2e59975Revert "Corrections to words."

comment:16 Changed 6 years ago by git

Commit: 2e599755176bcdd903f9c2fee015df7e32345b2534be2bb7941f2e6f343b9c88ef42e02d7ba77522

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

34be2bbTypo on variable name corrected.

comment:17 Changed 6 years ago by Jori Mäntysalo

I was blind.

comment:18 Changed 6 years ago by Paul Masson

Status: needs_reviewpositive_review

All doctests pass in combinat/words and combinat/posets

comment:19 Changed 6 years ago by Volker Braun

Branch: u/jmantysalo/vertex-color34be2bb7941f2e6f343b9c88ef42e02d7ba77522
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.