FIX: ensure type stability for missing cmaps in set_cmap
#23785
Conversation
Previously this would raise a `ValueError`, the refactor caused it to raise `KeyError`. This inadvertently located a bad test in xarray, but restoring the old behavior.
f8851ce to
0b6b385
Compare
|
FYI, @mwaskom; not sure if you'll want to keep this compat bit: https://github.com/mwaskom/seaborn/pull/2971/files#diff-c6ea5a328fda7cdae50968540e9b71734bbf30b191ef2d9b710b60847cacb9edR208 |
| cmap if cmap is not None else mpl.rcParams['image.cmap'] | ||
| ] | ||
| cmap_name = cmap if cmap is not None else mpl.rcParams["image.cmap"] | ||
| _api.check_in_list(sorted(_colormaps), cmap=cmap_name) |
There was a problem hiding this comment.
Do you want to put a comment in saying you added this check for error raising type stability so we don't remove it later? Does it need to be sorted or can it be list(_colormaps)?
There was a problem hiding this comment.
The input is used in formatting the error to tell the user what the allowed values are. Given the number of color maps I think sorting is a usability win / kindness for the user to discover what the correct spelling of the color map they tried to ask for is.
There was a problem hiding this comment.
I pushed a comment about why we are doing extra work.
…cmaps in `set_cmap`
…785-on-v3.6.x Backport PR #23785 on branch v3.6.x (FIX: ensure type stability for missing cmaps in `set_cmap`)
PR Summary
Previously this would raise a
ValueError, the refactor caused it to raiseKeyError. This inadvertently located a bad test in x-array, but restoringthe old behavior.
https://github.com/pydata/xarray/runs/8104117777?check_suite_focus=true#step:8:673
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).