Opened 5 years ago

Closed 5 years ago

#21048 closed enhancement (fixed)

Graph plotting: vertex colors

Reported by: jmantysalo Owned by:
Priority: major Milestone: sage-7.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:

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 5 years ago by jmantysalo

  • Branch set to u/jmantysalo/vertex-color

comment:2 Changed 5 years ago by jmantysalo

  • Cc paulmasson added
  • Commit set to b8e8b5c5a12cc94128a230bc2346a6f32d49ddb0
  • Status changed from new to needs_review

On a way to more customizable graph plotting.


New commits:

b8e8b5cModified setting of graph vertices colors.

comment:3 follow-up: Changed 5 years ago by paulmasson

  • 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?

Last edited 5 years ago by paulmasson (previous) (diff)

comment:4 in reply to: ↑ 3 Changed 5 years ago by jmantysalo

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 5 years ago by jmantysalo

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 follow-up: Changed 5 years ago by paulmasson

  • 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 jmantysalo

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 5 years ago by paulmasson

  • Cc ncohen 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 5 years ago by tscrim

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 5 years ago by paulmasson

  • Status changed from needs_review to positive_review

comment:11 Changed 5 years ago by vbraun

  • 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 git

  • Commit changed from b8e8b5c5a12cc94128a230bc2346a6f32d49ddb0 to 60b487d3101146d4b8b10f0ae5eec193613880cc

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

60b487dCorrections to words.

comment:13 Changed 5 years ago by jmantysalo

  • Status changed from needs_work to needs_review

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

comment:14 Changed 5 years ago by paulmasson

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 5 years ago by paulmasson (previous) (diff)

comment:15 Changed 5 years ago by git

  • Commit changed from 60b487d3101146d4b8b10f0ae5eec193613880cc to 2e599755176bcdd903f9c2fee015df7e32345b25

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

2e59975Revert "Corrections to words."

comment:16 Changed 5 years ago by git

  • Commit changed from 2e599755176bcdd903f9c2fee015df7e32345b25 to 34be2bb7941f2e6f343b9c88ef42e02d7ba77522

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

34be2bbTypo on variable name corrected.

comment:17 Changed 5 years ago by jmantysalo

I was blind.

comment:18 Changed 5 years ago by paulmasson

  • Status changed from needs_review to positive_review

All doctests pass in combinat/words and combinat/posets

comment:19 Changed 5 years ago by vbraun

  • Branch changed from u/jmantysalo/vertex-color to 34be2bb7941f2e6f343b9c88ef42e02d7ba77522
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.