-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
BUG: Convert qualitative colormaps to ListedColormap #3973
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
BUG: Convert qualitative colormaps to ListedColormap #3973
Conversation
PEP8 issue:
Tagged as 2.0 as a) it is coming 'soon' to change the default color map anyway b) independent of if you like it or not, this sort of change is the local working definition of backwards incompatible changes . |
Ok, I fixed the spacing issue. It was said that "dumping them in as if they were continuous was a mistake", so this could be considered a bugfix rather than a new feature, but 2.0 is fine. I like whatever you like. :) Any ideas on how to implement this correctly instead of using the dictionary label hack? Should they be in a separate dictionary than |
We are doing violence and breaking back-compatibility on colors so this is a good time to do this. Once someone relies on a bug it becomes a feature ;) Could you also add a note in whats_new and api_changes explaining this change? Would it be better to store the color map values in a npy file that we bundle rather than as text (which would deal with the |
Note that these colormaps (and flag) have uses: i am sometimes more intrested in value change of a data set, and these give a nice approximation of the derviate. |
Under a heading of version 2.0?
I would prefer they stay as text for transparency of where they come from, and ease of editing in situations like this. |
Yes, please create a new heading for 2.0 Fair enough re text vs npy files. On Wed, Mar 18, 2015 at 11:54 PM endolith notifications@github.com wrote:
|
@endolith Any chance of getting the whats_new/api_changes entries? Do you think it is worth including instructions on how to convert these back to the continuous versions? |
I started to, but wasn't sure about the format:
|
@endolith , we have new instructions on how to create these entries: https://github.com/matplotlib/matplotlib/blob/master/doc/api/api_changes/ and https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new |
Should this be rebased on to |
I'm still not sure what to say in API changes, though. You would still use the same commands to use these colormaps, no? |
Actually, this doesn't behave as I would expect it to.
I would expect this to interpolate with the LinearSegmentedColormap, which it does: but then with the ListedColormap, I expected it to use the colors as indexed, but that's not how it works: like I would expect the second row to be green then brown, the third to be green brown blue, and so on, with gray not showing up at all until the row is |
Try adding the kwarg |
That works:
Without the parentheses gives an AssertionError. This should raise an actual exception with a meaningful message, right?
|
Yes, you are correct. I think this is one of the arguments for using traitlets--that it will facilitate catching this sort of error early, and with a clear error message. As I understand it, assert is more of a developer's tool--a way of checking assumptions, not of catching users' errors. |
aa3307e
to
6a5abb5
Compare
(Oh nevermind that was fixed already) Also rebased and added notes to whatsnew and apichanges |
So should these be moved to the new _cm_listed.py file created by #4707? The intention of ListedColormap is not what I expected, but still the most appropriate for these sets of data. |
Since you're changing so many colour map lines, would it be possibly to wrap them so that all 3 elements of the tuple were on the same line instead of the haphazard wrapping that occurs right now? |
@QuLogic Can you point out exactly which line you're referring to? The ones I added have all 3 on the same line. |
You mean change the dicts to lists of RBG tuples? I could do that, but it should be a separate PR. I was thinking I should remove the |
Rewrote the data for ColorBrewer's qualitative colormaps and "tagged" them by wrapping them in a dictionary so that they are implemented as ListedColormap instead of LinearSegmentedColormap.
They are now ListedColormap; remove note about them being possibly removed. Add notes to What's New and API Changes. Also added reminder to update the colormap documentation if you add a new colormap or change an existing one
6a5abb5
to
80d48f2
Compare
I rewrote them all as RGB lists and force-pushed. |
I take it you also removed the other tweaks to put in a separate PR later, then? |
They were all in this range and overwritten, actually (except one I missed and will now push again) |
Changed to use LinearSegmentedColormap.from_list() for ColorBrewer maps, based on values in Brewer's Excel spreadsheet divided by 255.
80d48f2
to
411d0b3
Compare
I went through these with a small script to check the values and I don't see anything incorrect. The data in the ColorBrewer section is much neater now. I don't see why this can't be merged and backported (or maybe re-opened directly to 2.x.) |
API: Convert qualitative colormaps to ListedColormap
Over a year, but it got in! |
API: Convert qualitative colormaps to ListedColormap
backported to v2.x as c27dd78 |
"Qualitative" colormaps are designed to be used on things like chloropleths, not smoothly-varying heatmaps, in applications such as scipy.ndimage.measurements.label. It would be nice to have some predefined named maps for this purpose. ColorBrewer has some (Accent, Dark2, Paired, Pastel1, Pastel2, Set1, Set2, Set3) but they were mistakenly created as LinearSegmentedColormap when they should have been ListedColormap, so I converted them.
I left these colormaps in the same _cm.py file and "tagged" them with a dictionary and hacked in a path for them to be implemented as ListedColormap. This is hacky and I expect you to reject it, but I don't know of a better way right now. Please suggest a more proper way to implement this, moving them to their own file, etc.
In #881 there was concern that this would cause backward compatibility problems, but this won't actually "break" any existing code that uses them, they can still be used for heatmaps, the images will just become solid colors instead:
They're pretty ugly when used as heatmaps, so I doubt they are being used this way very often.