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

BUG: Convert qualitative colormaps to ListedColormap #3973

Merged
merged 3 commits into from Feb 18, 2016

Conversation

endolith
Copy link
Contributor

@endolith endolith commented Jan 7, 2015

"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:

Paired original
Paired ListedColormap

They're pretty ugly when used as heatmaps, so I doubt they are being used this way very often.

@tacaswell tacaswell added this to the v2.0 milestone Jan 7, 2015
@tacaswell
Copy link
Member

PEP8 issue:

AssertionError: Found code syntax errors (and warnings):
/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/cm.py:56:25: E203 whitespace before ':'

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 .

@tacaswell tacaswell modified the milestones: next major release, Color overhaul Feb 17, 2015
@endolith
Copy link
Contributor Author

endolith commented Mar 8, 2015

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 datad?

@endolith endolith changed the title BUG: Convert qualitative colormaps to ListedColormap WIP: BUG: Convert qualitative colormaps to ListedColormap Mar 8, 2015
@tacaswell
Copy link
Member

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 1.000000000001 issues you cleaned up a bit more elegantly.

@Tillsten
Copy link
Contributor

Tillsten commented Mar 9, 2015

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.

@endolith
Copy link
Contributor Author

Could you also add a note in whats_new and api_changes explaining this change?

Under a heading of version 2.0?

Would it be better to store the color map values in a npy file that we bundle rather than as text

I would prefer they stay as text for transparency of where they come from, and ease of editing in situations like this.

@tacaswell
Copy link
Member

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:

Could you also add a note in whats_new and api_changes explaining this
change?

Under a heading of version 2.0?

Would it be better to store the color map values in a npy file that we
bundle rather than as text

I would prefer they stay as text for transparency of where they come from,
and ease of editing in situations like this.


Reply to this email directly or view it on GitHub
#3973 (comment)
.

@tacaswell
Copy link
Member

@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?

@endolith
Copy link
Contributor Author

I started to, but wasn't sure about the format:

doc/users/whats_new.rst:

.. _whats-new-2-0:

new in matplotlib-2.0
=====================

Colormaps
---------
Colorbrewer's "qualitative" colormaps ("Accent", "Dark2", "Paired", 
"Pastel1", "Pastel2", "Set1", "Set2", "Set3") are now implemented
as ``ListedColormap`` instead of ``LinearSegmentedColormap``.
This way, they can be used for maps such as chloropleths or
labeled images.

.. _whats-new-1-5:

doc/api/api_changes.rst:

Changes in 2.0.x
================

???

Changes in 1.4.x
================

@WeatherGod
Copy link
Member

@endolith
Copy link
Contributor Author

Should this be rebased on to color_overhaul?

@endolith
Copy link
Contributor Author

I'm still not sure what to say in API changes, though. You would still use the same commands to use these colormaps, no?

@endolith
Copy link
Contributor Author

Actually, this doesn't behave as I would expect it to.

for n in xrange(12):
    subplot(11, 1, n)
    imshow([arange(n)], cmap='Dark2', interpolation='nearest')
    plt.axis('off')

I would expect this to interpolate with the LinearSegmentedColormap, which it does:

linear segmented

but then with the ListedColormap, I expected it to use the colors as indexed, but that's not how it works:

listed colormap

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 [0, 1, 2, 3, 4, 5, 6, 7]. I'm misunderstanding ListedColormap?

@efiring
Copy link
Member

efiring commented Jul 18, 2015

Try adding the kwarg norm=mpl.colors.NoNorm. That will pass the integers through for indexing. Otherwise the default Normalize will scale them just like any other sequence.

@endolith
Copy link
Contributor Author

That works:

for n in xrange(12):
    plt.subplot(11, 1, n)
    plt.imshow([arange(n)], cmap='Dark2', norm=colors.NoNorm(),
                interpolation='nearest')
    plt.axis('off')

that works

Without the parentheses gives an AssertionError. This should raise an actual exception with a meaningful message, right?

  File "C:\Python27\lib\site-packages\matplotlib\axes\_axes.py", line 4636, in imshow
    assert(isinstance(norm, mcolors.Normalize))

AssertionError

@efiring
Copy link
Member

efiring commented Jul 18, 2015

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.

@endolith
Copy link
Contributor Author

(Oh nevermind that was fixed already)

Also rebased and added notes to whatsnew and apichanges

@endolith
Copy link
Contributor Author

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.

@tacaswell tacaswell removed this from the Color overhaul milestone Oct 26, 2015
@tacaswell tacaswell modified the milestones: next major release (2.0), Color overhaul Oct 26, 2015
@QuLogic
Copy link
Member

QuLogic commented Dec 3, 2015

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?

@endolith
Copy link
Contributor Author

endolith commented Dec 4, 2015

@QuLogic Can you point out exactly which line you're referring to? The ones I added have all 3 on the same line.

@QuLogic
Copy link
Member

QuLogic commented Dec 4, 2015

Here's one for example from 4d547af.

@endolith
Copy link
Contributor Author

endolith commented Dec 4, 2015

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 0.900000000000000020.9 tweaks to a separate PR anyway, for clarity.

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
@endolith
Copy link
Contributor Author

endolith commented Dec 4, 2015

I rewrote them all as RGB lists and force-pushed.

@QuLogic
Copy link
Member

QuLogic commented Dec 4, 2015

I take it you also removed the other tweaks to put in a separate PR later, then?

@endolith
Copy link
Contributor Author

endolith commented Dec 4, 2015

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.
@QuLogic
Copy link
Member

QuLogic commented Feb 18, 2016

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.)

tacaswell added a commit that referenced this pull request Feb 18, 2016
API: Convert qualitative colormaps to ListedColormap
@tacaswell tacaswell merged commit 18169b2 into matplotlib:master Feb 18, 2016
@tacaswell
Copy link
Member

Over a year, but it got in!

tacaswell added a commit that referenced this pull request Feb 18, 2016
API: Convert qualitative colormaps to ListedColormap
@tacaswell
Copy link
Member

backported to v2.x as c27dd78

@endolith endolith deleted the Qualitative_ListedColormap branch February 18, 2016 23:53
@endolith endolith changed the title WIP: BUG: Convert qualitative colormaps to ListedColormap BUG: Convert qualitative colormaps to ListedColormap May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants