added alias to gray and grey match same colormaps #26003
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
|
Welcome, @madeira-dev ! I believe the CI failures are unrelated here. |
|
Hi @melissawm! |
lib/matplotlib/cm.py
Outdated
| @@ -46,6 +46,13 @@ def _gen_cmap_registry(): | |||
| for cmap in list(cmap_d.values()): | |||
| rmap = cmap.reversed() | |||
| cmap_d[rmap.name] = rmap | |||
|
|
|||
| # Register colormap aliases for gray and grey. | |||
| cmap_d['grey'] = cmap_d['gist_grey'] = cmap_d['gray'] | |||
There was a problem hiding this comment.
Let's not make assumptions on equivalence and instead plainliy assign by the naming alias.
| cmap_d['grey'] = cmap_d['gist_grey'] = cmap_d['gray'] | |
| cmap_d['grey'] = cmap_d['gray'] | |
| cmap_d['gist_grey'] = cmap_d['gist_gray'] |
Please also move this section to before "Generate reversed cmaps" so that you do not need to handle the *_r cases yourself.
|
Can you please revert the changes of ' to "? And the additional empty lines. Probably your editor did this. Will be much easier to review the actual changes plus that we prefer to avoid superfluous changes like this. |
lib/matplotlib/cm.py
Outdated
|
|
||
| # Register colormap aliases for gray and grey. | ||
| cmap_d["grey"] = cmap_d["gray"] | ||
| cmap_d["Greys"] = cmap_d["Grays"] |
There was a problem hiding this comment.
| cmap_d["Greys"] = cmap_d["Grays"] | |
| cmap_d["Grays"] = cmap_d["Greys"] |
This one, (confusingly, which is what this PR is addressing) is actually defined with an "e" already, so the "a" alias needs to be made, unlike the others.
This is causing import errors (and thus other CI errors)
There was a problem hiding this comment.
Oh okay, I will invert this aliasing and also fix the problems @oscargus mentioned above (it surely happened due to my editor and I just didn't noticed)
lib/matplotlib/cm.py
Outdated
|
|
||
| # Register colormap aliases for gray and grey. | ||
| cmap_d["grey"] = cmap_d["gray"] | ||
| cmap_d["gist_grey"] = cmap_d["gist_gray"] |
There was a problem hiding this comment.
| cmap_d["gist_grey"] = cmap_d["gist_gray"] | |
| cmap_d["gist_grey"] = cmap_d["gist_gray"] | |
| cmap_d['gist_yerg'] = cmap_d['gist_yarg'] |
Let's take this as well - It's less likely that somebody will use the other spelling in the reverse spelling, but it does not hurt to have this as well?
lib/matplotlib/cm.py
Outdated
| # Someone may set the extremes of a builtin colormap and want to register it | ||
| # with a different name for future lookups. The object would still have the | ||
| # builtin name, so we should update it to the registered name | ||
| if self._cmaps[name].name != name: | ||
| self._cmaps[name].name = name |
There was a problem hiding this comment.
Where does this come from? It seems unrelated to the topic of the PR?
There was a problem hiding this comment.
I'll add the alias for the reverse spelling.
Sorry about this unrelated modification, I didn't notice it and can't see where it came from, I'll remove it.
PR summary
Closes #8965
As suggested in the issue discussion, added alias to match the different spellings of gray and grey in the end of _gen_cmap_registry function in cm.py
PR checklist