-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Make new colormaps full-fledged citizens #5283
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
Conversation
Not sure if I am happy with the "reduce" name. Makes me think of python's reduce() or numpy's reduce(). |
Maybe "segment"? |
Eh... better, but not really clear. Maybe a better docstring would help in explaining its purpose. To me, it just looks like a glorified copy constructor... |
It's a copy constructor that changes the number of lookup table entries. Maybe "resample"? |
Much better! |
Updated with renamed methods. |
by the way... pet peeve of mine is the error message below that prints out the list of keys. The keys aren't sorted, so it changes all the time and it is hard to find the one I want if I misremember the name. Think we can sneak a fix of that here? |
Keys are now sorted in the error message. |
+1 from me |
do we need a "whats_new"? |
I don't think this needs a what's new -- it's basically just a bugfix that should have been done when the new colormaps were added (and there's already a what's new for those). |
I was more referring to the introduction of a new colormap method. Unless you want to make it private for now? |
I'll just make these private for now. The interface to this is really |
alright, so it is now +1 from me. Was this intended to get backported to v1.5.x? |
@WeatherGod yes I think so #5202 is tagged at 1.5.0 |
Make new colormaps full-fledged citizens
Make new colormaps full-fledged citizens
backported to v1.5.x as e7e0807 |
Fix #5202