seaborn icon indicating copy to clipboard operation
seaborn copied to clipboard

color_palette has incompatible desat and as_cmap arguments

Open tfardet opened this issue 2 years ago • 5 comments

Issue

Right now, with seaborn 0.11, the desat and as_cmap arguments are incompatible, leading to errors (see "Error messages" section for details).

This is because in color_palette already calls the other functions (e.g. hsl_palette or mpl_palette) with as_cmap set to True, leading on a non-iterable colormap object before the use of desat.

Potential solution

A possible fix is to call the intermediate functions with as_cmap=False, using the original number of colors for matplotlib colormaps, call desat on the list of colors, then convert it back to a colormap.

If this solution is considered appropriate, I can make a PR to fix it.

Error messages

For seaborn palettes:

cmap = sns.color_palette("hls", desat=0.1, as_cmap=True)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-13-d79473143e6f> in <module>
----> 1 cmap = sns.color_palette("hls", desat=0.1, as_cmap=True)

~/.local/lib/python3.10/site-packages/seaborn/palettes.py in color_palette(palette, n_colors, desat, as_cmap)
    209 
    210     if desat is not None:
--> 211         palette = [desaturate(c, desat) for c in palette]
    212 
    213     if not as_cmap:

TypeError: 'ListedColormap' object is not iterable

And for matplotlib colormaps:

cmap = sns.color_palette("Blues", n_colors=100, desat=0.1, as_cmap=True)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-4a13dee9dc6e> in <module>
----> 1 cmap = sns.color_palette("Blues", n_colors=100, desat=0.1, as_cmap=True)

~/.local/lib/python3.10/site-packages/seaborn/palettes.py in color_palette(palette, n_colors, desat, as_cmap)
    209 
    210     if desat is not None:
--> 211         palette = [desaturate(c, desat) for c in palette]
    212 
    213     if not as_cmap:

TypeError: 'LinearSegmentedColormap' object is not iterable

Related issues

#2290 #2694

tfardet avatar Jan 21 '22 13:01 tfardet

I don’t think this is worth supporting, I think it should just give clear error that they’re incompatible.

mwaskom avatar Jan 21 '22 13:01 mwaskom

May I ask why not? This is fairly trivial and does not require any major change to the actual code

tfardet avatar Jan 21 '22 14:01 tfardet

I don’t think your proposed solution would properly handle qualitative palettes or the fact that mpl_palette truncates the range it samples discrete colors from.

more generally, desat is kinda hacky and isn’t something I’d add at this point.

mwaskom avatar Jan 21 '22 14:01 mwaskom

OK, I thought about what you said and did some more digging on seaborn palettes and came out with the following picture:

  1. currently, asking for a matplotlib palette with color_palette or mpl_palette, it is impossible to obtain an object that is equivalent to the original colormap (see example below) because, as you said, the range is truncated
  2. it is nonetheless possible to make desat work with all palettes, i.e. matplotlib linear and discrete, seaborn, and h(u)sl

The current situation leads to behavior that feel somewhat undesirable in my opinion. For instance, the following code leads to two different colorings:

import matplotlib.pyplot as plt
import pandas as pd
import numpy as np

import seaborn as sns


rng = np.random.default_rng()

data = {"A": rng.integers(10, size=100), "B": rng.integers(10, size=100), "C": rng.integers(3, size=100)}

df = pd.DataFrame(data)

cmap = sns.color_palette("Blues", as_cmap=False)

fig, (ax1, ax2) = plt.subplots(1, 2)

sns.boxplot(data=df, x="A", y="B", hue="C", palette=cmap, ax=ax1)
sns.boxplot(data=df, x="A", y="B", hue="C", palette="Blues", ax=ax2)

plt.show()

image

So given these two facts, I see the following possibilities (I've tested and implemented each of them so they are all technically possible):

  • leave mpl_palette as it is now and update color_palette to return a full colormap and handle LinearSegmentedColormap inputs separately (this has the drawback of making mpl_palette and color_palette return different objects when asked to return a matplotlib palette with default arguments)
  • update mpl_palette so that it returns the full palette by default (n_colors=None and we use cmap.N in the function) and have color_palette handle LinearSegmentedColormap inputs separately (this changes default behavior but I would recommend this solution)

There are minor subtleties but I'll leave them for later discussions if we decide to go for one of the two issues. These two implementations do not change the behavior for discrete colormaps/seaborn palettes and the only palettes that would lead to two different colorings with the code I posted would be h(u)sl, which is almost anavoidable unless we also check for palette name and override the palette if its name is h(u)sl.

Independantly of this, I would suggest to update the color_palette to support desat with as_cmap=True: I understand that you now feel desat is hacky but it already exists and making it work for colormaps requires only tiny changes to the code that are very unlikely to impact maintainability. Furthermore, not supporting anyways require to change the function so that the error code is explicit (as seen from several issue reports).

tfardet avatar Jan 22 '22 17:01 tfardet

The current situation leads to behavior that feel somewhat undesirable in my opinion. For instance, the following code leads to two different colorings

I think this is unrelated to the question about desat, and it is not an example of color_palette doing the wrong thing. Rather, this is an example of color_palette doing the right thing and it producing a potentially surprising result in a particular context. You asked color_palette for a list of 6 colors and that is what it gave you; you then gave boxplot a list of colors and it assigned them to the different levels of the hue variable, as it should.

The change to be made here is to fire a warning when the palette is a list of colors that is longer than the number of hue levels. See https://github.com/mwaskom/seaborn/issues/2662 for more context.

(update mpl_palette so that it returns the full palette by default ... this changes default behavior but I would recommend this solution)

This would be a breaking change to a public (although probably rarely used) function with no deprecation path, so it really can't be considered unless there's a strong argument for it, and I don't see one here.

Furthermore, not supporting anyways require to change the function so that the error code is explicit

Yes, but adding a two line conditional / exception and a simple test does not require thinking through all the tradeoffs and potential downstream consequences, nor does it preclude enabling the feature in the future.

Anyway, another factor to consider here is that working around this in user-space is pretty simple, e.g.

mpl.colors.ListedColormap(sns.color_palette("Blues", 256, desat=.3))

image

mwaskom avatar Jan 22 '22 19:01 mwaskom