DOC: Change colormaps for examples (showcase different types, discourage 'jet') #921
Conversation
…verging colormaps for bipolar data
|
I think this is a really beneficial change. I'd like to get an opinion from someone a little more invested in colormaps than I before merging. |
|
Running backend_bases.py to generate all the examples reveals several places where an import of cm is missing, so the example fails. |
|
Ok, should I add |
|
On 06/07/2012 03:56 PM, endolith wrote:
If the rest of the example is using the "plt.contourf" etc. style, then Eric |
| @@ -35,14 +35,14 @@ | |||
| # pcolor plot. | |||
| plt.figure() | |||
| plt.gca().set_aspect('equal') | |||
| plt.tripcolor(triang, z, shading='flat') | |||
| plt.tripcolor(triang, z, shading='flat', cmap = plt.cm.rainbow) | |||
| plt.colorbar() | |||
There was a problem hiding this comment.
You need to import cm in this example
There was a problem hiding this comment.
I don't understand. matplotlib.pyplot does a from matplotlib import cm, so plt.cm.rainbow should be ok?
There was a problem hiding this comment.
It's OK now, but it wasn't when I added my line note. It was fixed in endolith@e1b17ec#diff-7. I do think it better to explicitly import cm rather than use it from pyplot though, because it will educate users where they should go to look at the module docs and code
|
I'm all for it. We inherited the default jet colormap from matlab. Most psychophysicists discourage it, so I'm happy to see someone who cares enough about it to make some intelligent suggestions. It will give some nice visual diversity to our gallery, encourage better habits in users who are exploring through the examples, and give a lot more exposure to many users to the incredible diversity of colormaps we ship. Thanks also for the docstring and comment cleanups in the examples. |
|
Please do double-check that I made good choices, though. I'm no psychophysicist. For instance, in tricontour_vs_griddata.py, I initially chose a diverging colormap with a white middle, but then realized the white middle hid the difference between the two plots in the lower left corner, which is probably supposed to be visible in this example. |
|
Interested to hear others opinions on this, but maybe we could just accept strings as an argument to cmap? The following example seems simple and intuitive: |
|
On 06/13/2012 04:31 AM, Phil Elson wrote:
This idea has also occurred to me, but I never did anything about it. Eric |
|
Doesn't it already accept the name directly? Works for me |
|
On 06/13/2012 08:51 AM, endolith wrote:
But not for contour, which not only does not support this, but is using Eric |
|
I think it would be great to support strings everywhere, including contour -- and we can fix those asserts while we're at it. @endolith: Did you realise what a mess you were opening up when you started this? :) |
|
So each plotting function would import cm internally? Would they recognize strings for colormaps created by register_cmap? Well the vmid centered norm and the string cmaps should be separate issues, no? |
|
On 06/15/2012 11:31 AM, endolith wrote:
Yes.
Yes, definitely separate from your pull request. Is your request Eric |
| @@ -39,14 +39,14 @@ | |||
| gridsize=30 | |||
|
|
|||
| plt.subplot(211) | |||
| plt.hexbin(x,y, C=z, gridsize=gridsize, marginals=True) | |||
| plt.hexbin(x,y, C=z, gridsize=gridsize, marginals=True, cmap=plt.cm.RdBu, vmax=abs(z).max(), vmin=-abs(z).max()) | |||
There was a problem hiding this comment.
Ideally the line should be wrapped to 80 characters.
|
I think this is a really beneficial change and I appreciate you taking the time to do it. I think my preference, seen as we are changing so many of the examples, would be to use strings to specify the colormap as this would seem to be a preferable solution to using the I have opened a PR (#951) to get this functionality, which, if @efiring agrees, I think should be a dependency on this PR. @efiring: thoughts? |
| cset = ax.contour(X, Y, Z, zdir='z', offset=-100) | ||
| cset = ax.contour(X, Y, Z, zdir='x', offset=-40) | ||
| cset = ax.contour(X, Y, Z, zdir='y', offset=40) | ||
| cset = ax.contour(X, Y, Z, zdir='z', offset=-100, cmap = cm.coolwarm) |
There was a problem hiding this comment.
Stylistically, throughout this PR it would be preferable to not put spaces between function/method keyword argument definitions. e.g.
cset = ax.contour(X, Y, Z, zdir='z', offset=-100, cmap = cm.coolwarm)
becomes
cset = ax.contour(X, Y, Z, zdir='z', offset=-100, cmap=cm.coolwarm)
|
I think this set of changes has gone through enough review that it makes sense to commit it (as I did just now), and leave any changes related to #951 to another pull request. There is no need to make all examples use the string form of colormap specification, though certainly this should be illustrated. |
|
Ok, should anything be done for other examples like http://docs.scipy.org/doc/scipy/reference/tutorial/interpolate.html#id1 ? |
|
I don't understand. What would you do with that example? It looks OK to me, given that it is illustrating scipy functionality. Are you referring to the use of cm.jet in the example at the bottom of that page? |
|
Yes, there are other examples outside of the gallery that are using cm.jet, probably just because of its ubiquity. Should I go around changing those, too? |
|
You are welcome to propose such changes in the matplotlib docs. Scipy might also be receptive, but that involves different mailing lists, repos, etc. |
Here's a bunch of colormap-related changes to the examples for Issue #875, intended to
There are more that could be changed (like this), but I imagine even this set of changes needs discussion first.
(Also some typo and spelling fixes, which I probably should have done in a different branch, but I'm still learning how this git thing works)