Update label_color_mapping and inherit palette from plot widget #49
Conversation
…t inherit plot palette by default
There was a problem hiding this comment.
I'll word the changes I wish for a little differently, at the risk of repeating myself from the local comments. I understand the color and palette parameters as having a natural order:
- Colors for named labels in the
label_color_mappingargument should be assigned as is. - Further labels unnamed should be assigned colors from the palette provided through the
paletteargument. The order does not matter, but it must be stored so that it is sustained in any linked label editor widget. - If there are more unique labels (aka label names) than the sum of the sizes of
label_color_mappingandpalette, or ifpalette_sizeis larger than that sum, then we should enlarge the palette obtained by this concatenation usingglasbey.extend_palette, granting new colors determined to maximize the visual variance of the final palette.
If a label editor widget is, in turn, created with explicit factors and colors (to build its own view of the palette), then it must strictly match that of the plot. This would suggest that the user did their own palette homework and that no automatic palette extension occurred. Any palette incoherence between the plot and the editor should be punished with an exception. :-) No trying to fix a mess made by the user.
thisnotthat/bokeh_plot.py
Outdated
| for label, color in label_color_mapping.items(): | ||
| factors.append(label) | ||
| colors.append(color) | ||
| for factor, color in zip(factors, self._base_palette): |
There was a problem hiding this comment.
So if the user provides a palette that is smaller than the cardinal of the label set, then the resulting plot will have less colours than there are unique labels. I believe this is a bad surprise to subject the user to: it looks like a bug.
thisnotthat/bokeh_plot.py
Outdated
| factors.append(label) | ||
| colors.append(color) | ||
| for factor, color in zip(factors, self._base_palette): | ||
| if factor in label_color_mapping: |
There was a problem hiding this comment.
This approach makes it so the final palette is not composed in order to maximize the palette's visual variance, given the colors chosen by the user. I wish that if I provided parameters label_color_mapping={"my-special-label": "#abcdef"} and palette=None, then the plot would use glasbey.extend_palette to complete my choice. Same if a palette is provided by palette=[...], the the concatenation of the color mapping and the palette is too short given the number of unique labels.
If the user provides both a color map and a palette list, we should work out of the concatenation of the two. It's the user's responsibility to make these visually variant. Any further addition to the palette (to take into account a larger required palette size or number of unique labels) should be obtained by glasbey.extend_palette.
Turn label_color_mapping into a partial map and have LabelEditorWidget inherit plot palette by default