@efiring: Seems like something (contours and coloring) that you might have some input about.

I think I see what @kthyng is trying to fix, but to be sure, it would be very helpful to have a minimal runnable example. I think the present changeset might be changing more than it needs to, and might have undesirable side effects.

I helped out @kthyng with this and she created a notebook with some pictures.

@kthyng Would you mind pasting those here? You can drag png files right into the comment box and github does magic to embed them for you.

Sorry, I forgot to write a comment in here too. I am learning how to properly contribute to a community code like this.

This fix does alter the data, z, itself, but from what I could tell, a similar sort of thing happens in the pcolor function when a vmin/vmax is applied. I also thought about trying to implement the change in the colorbar code since it is more of a colorbar problem really, but upon tracking back through the functions, I ended up changing it in contourf instead.

The bug being addressed definitely needs fixing but we definitely need a test. I can work with you on that if you need a hand.

Thanks everyone! With help, I added in a couple of tests to test_contour.py. I've tried to provide the useful, relevant information, but I may have missed some.

@kthyng, thank you for contributing, please don't be discouraged by my blunt criticism and questions.

It is not at all clear to me that this is a bug, and if this change in behavior is indeed desired, then I don't think it is being changed in the right part of the code. This is not a colorbar question, it is a question of default choice of levels, isn't it?

Consistently throughout mpl, vmin and vmax pertain to the color mapping, no more, no less. That is how they are presently being used in contourf. They are fed to the norm.

What it seems you want to do is to overload them, so that when no contour levels are specified, but they are auto-generated based on a desired number of levels, then they will be based on vmin and vmax, if present, instead of zmin and zmax. If this is a good thing to do (debatable, but I am not necessarily opposed), then it should be done within the code that performs this auto-generation of levels, not where it is in the present PR.

In addition, modifying the input array is bad practice. If we are doing that elsewhere, then those places are bugs to be fixed.

On Jun 29, 2013 4:47 PM, "Eric Firing" notifications@github.com wrote:

@kthyng, thank you for contributing, please don't be discouraged by my
blunt criticism and questions.

It is not at all clear to me that this is a bug, and if this change in
behavior is indeed desired, then I don't think it is being changed in the
right part of the code. This is not a colorbar question, it is a question
of default choice of levels, isn't it?

Consistently throughout mpl, vmin and vmax pertain to the color mapping,
no more, no less. That is how they are presently being used in contourf.
They are fed to the norm.

What it seems you want to do is to overload them, so that when no contour
levels are specified, but they are auto-generated based on a desired number
of levels, then they will be based on vmin and vmax, if present, instead of
zmin and zmax. If this is a good thing to do (debatable, but I am not
necessarily opposed),

I think that this sort of carefully considered overloading of vmin and vmax
might be desirable. What happened before wasn't undesirable, but this might
be better. Probably should have some more examinations of before/after
images with other kwargs such as extend.

then it should be done within the code that performs this auto-generation
of levels, not where it is in the present PR.

Agreed.

In addition, modifying the input array is bad practice. If we are doing
that elsewhere, then those places are bugs to be fixed.

Definitely agreed! I have been considering extending the test suite to do
before/after comparisons of input data, but it might have to wait for now.

—
Reply to this email directly or view it on GitHub.

That makes sense about it not necessarily being the correct place to make the change.

It probably makes sense to first decide whether or not the change is actually desired by people in general, so I came up with another code snippet with the intention of illustrating why this problem comes up in my research (though, just because it comes up for me doesn't necessarily mean that it should change!). I often have 4-dimensional arrays (x,y,z,t) that I want to plot. I will plot x and y along the x and y axes and z as the color (in pcolor or contourf or similar), and then I will make a different snapshot for those variables for each time step. I want the colorbar and the coloring for every snapshot to be set by the maximum and minimum values of the entire 4D array so that I can make an animation. If the colorbar changes every snapshot, the animation looks terrible. This works fine when I used pcolor but I often like to use different plot functions for different emphases and thus I wanted to have the same behavior with contourf. See below for code and plots. I didn't make an animation, but I just meant to show that as time changes, the colorbar will change between snapshots using the original code.

from matplotlib import pyplot as plt
import numpy as np

xvec = np.linspace(0,10)
yvec = np.linspace(0,20)
X, Y = np.meshgrid(xvec,yvec)
Z1 = X**2 + Y**2
Z2 = X**2 + Y**2 + 300
datamax = max(Z1.max(),Z2.max())

fig = plt.figure(figsize=(10,5))
fig.add_subplot(111)
plt.subplot(2,2,1)
plt.pcolor(X,Y,Z1,cmap='pink_r',vmax=datamax)
plt.title('pcolor plot, time 1')
plt.colorbar()

plt.subplot(2,2,2)
plt.pcolor(X,Y,Z2,cmap='pink_r',vmax=datamax)
plt.title('pcolor plot, time 2')
plt.colorbar()

plt.subplot(2,2,3)
plt.contourf(X,Y,Z1,cmap='pink_r', vmax=datamax)
plt.title('contourf plot, time 1')
plt.colorbar()

plt.subplot(2,2,4)
plt.contourf(X,Y,Z2,cmap='pink_r', vmax=datamax)
plt.title('contourf plot, time 2')
plt.colorbar()

plt.suptitle('Without Fix')
plt.tight_layout()
plt.savefig('comp_without_fix.png',bbox_inches='tight')

@kthyng, my preferred method for handling this situation is to explicitly decide beforehand what contour levels I want all the plots to use, and then specify them via the "levels" kwarg. Then there is no need for the vmin, vmax kwargs, unless one wants to tweak the mapping.

Oops, I forgot to set vmin on that example.

@efiring, yes, that is what I do currently. Explicitly setting the levels takes a lot of work, though, if you have to do guess and check to get nice levels. Automatically setting the levels from a desired min to max value isn't a great solution because the numbers on the levels end up ugly. So, I have been using ticker.MaxNLocator() to get nice numbers out. I am remembering that this ends up actually being a very specific problem if I go through all the things I have tried over the past few months. I think the ticker solution ends up being fine (though it is a few extra steps, the way I have been doing it) if I have a sequential data set that I want to plot. However, I often plot diverging data sets and I want to be able to control the actual number of contours used because with a diverging data set I want to have one contour level around 0 and an equal number of contours extending in the positive and negative directions from zero. With MaxNLocator() I haven't figured out a way to do this because it chooses the exact number of levels, not exceeding the user's input value.

I hope I am remembering this correctly from my work. The sprint time at the SciPy conference is ending so I can't make an example up currently, but I could later if that would help. I also do still think that even if there is a way to get my desired behavior currently (without having to do guess and check to get nice contour levels), having to go through a lot of steps for this type of plot is undesirable, but perhaps it is just unavoidable. I currently have been using the MaxNLocator() solution and since I can't make it have the specific number of contours I want (which would be a small, odd number, like 9), I have been using many contour levels so that the effect is less noticeable. Hopefully this makes sense.

@efiring I think there are two issues that a fix here would ideally address:

1. Make it easy to get what to me is desirable behavior with contourf plots, which is to have the color bar be consistent between plots when one wants to set a vmin/vmax. Currently one has to explicitly choose the contour levels either by guess and check (to get nice numbers) or using the tickers (as explained here: http://matplotlib.1069221.n5.nabble.com/question-about-contours-and-clim-td21111.html), which seems like too much work for a simple plot task (though this is what I currently do). The fix I submitted was intended to address this, but it sounds like did not do so in a reasonable way.
2. One thing that I have not been able to make happen (and my fix does not address) is to be able to simultaneously control the actual number of contours in a contourf plot and the min/max values without having to explicitly set the levels by hand. Using a ticker did not work for me in this case because I wanted to set a small, odd number of levels so that the middle level is evenly situated around 0 in a diverging data set. The closest I could get was setting a small number of levels but not being able to control it being an odd number of levels, using MaxNLocator(). I am currently making plots instead by using many levels so that the effect of having data near zero displayed as one of the diverging colors or the other (which is often not very meaningful) is diluted. This works but is not my first choice.

I would have to think more about how these changes could be put into effect if others thought these changes would be a good addition to matplotlib.

Ok, a lot of valuable information is in this PR, but as it stands I don't think we can merge it, so I'm going to go ahead and close the PR. Hopefully this is the beginning of a conversation which allows us to improve the contour api, particularly for cases where multiple subplots are being drawn. Perhaps that is a conversation to have on the mailing-list for now.

Thanks!