Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print list of palettes on invalid palette choice #3535

Closed
wants to merge 3 commits into from
Closed

Print list of palettes on invalid palette choice #3535

wants to merge 3 commits into from

Conversation

beyarkay
Copy link
Contributor

(Just a small thing, feel free to reject.)

This PR simply prints out some valid colour palettes if the user provides an invalid one. It doesn't print out every palette.

@mwaskom
Copy link
Owner

mwaskom commented Oct 21, 2023

Thanks for this, but I'm not sure it's helpful to provide a list of valid palettes that doesn't include all valid palettes.

And even though it doesn't print every palette (seaborn palette specification is programmatic and that wouldn't really be possible), it sure does print a lot of them. That produces a huge error message and I'm not sure the UX of searching through it is really a net benefit. Things would look even more overwhelming if e.g. you were using colorcet which adds several dozen additional palettes.

If the objective here is to help people who misspell a colormap (and I agree, it can be really hard to remember the exact spelling of those colorbrewer maps), I'd be in favor an approach that did a fuzzy match with a suggestion, similar to what Python is trying to do with its attribute errors lately. (E.g. "No palette named 'YlGrBu', did you mean 'YlGnBu'"?) Though of course that is trickier to implement!

@beyarkay
Copy link
Contributor Author

Yeah I did underestimate the number of available colour palettes a bit 😅 and looking at it with fresh eyes I'd agree that the massive list is a bit much and not super helpful.

I don't think seaborn has a dependency on a fuzzy matching library (correct me if I'm wrong) and I'm not sure I have time to implement one myself at the moment. It feels a bit overkill to bring in a dependency just for this error message, unless you think it would be useful to have in the future (?).

An alternative would be to link the user to either the seaborn "Choosing color palettes" docs or the matplotlib "Colormap reference".

@mwaskom
Copy link
Owner

mwaskom commented Oct 22, 2023

I don't think seaborn has a dependency on a fuzzy matching library (correct me if I'm wrong) and I'm not sure I have time to implement one myself at the moment.

Correct, and I am also -1 on adding a new dependency on a third party library just for this. But Python itself is making these kind of suggestions as of 3.11 so it is worth looking into how that works and whether there is something in the stdlib that could be leveraged. What I know of off the top of my head is difflib, and a simple PoC using that is straightforward:

import difflib
arg = "YlGrBu"
scores = pd.Series({name: difflib.SequenceMatcher(a=arg, b=name).ratio() for name in mpl.colormaps})
scores.sort_values(ascending=False).head()
YlGnBu      0.833333
YlGnBu_r    0.714286
YlGn_r      0.666667
YlOrBr      0.666667
RdYlBu      0.666667

I am not certain that this is the best way to do it, and there are a number of questions about how exactly to go about implementing this in the right way (e.g. you'd probably want a threshold on the score, things like _r should get special handling, etc.).

@mwaskom
Copy link
Owner

mwaskom commented Dec 3, 2023

I'm going to close this PR but happy to continue discussion of improved suggestions in an issue if interested in following up.

@mwaskom mwaskom closed this Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants