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

Adjustments to contour.py so that levels are set accordingly to input vmin and vmax, if provided, so color bar works correctly. #2176

Closed
wants to merge 4 commits into from

Conversation

kthyng
Copy link
Contributor

@kthyng kthyng commented Jun 29, 2013

Previously, if a user wanted to include a vmin/vmax value to a call to contour or contourf, the plot itself would reflect the desired color range, but if a color bar was added to the plot, it would not nicely reflect the desired data range. This change fixes that problem by adding in a check for vmin and vmax in _contour_args() as input arguments and uses the values to set the levels in the data according to the desired data range. This works for whether the desired data range (from vmin/vmax) are within or without the actual data range, or whatever combination of the two. With this change, a plot made with pcolor and a color bar and a plot made with contourf and a color bar will behave similarly. The actual max and min boundary limits for the contour colorbars will not necessarily be equal to the input vmin and vmax because contour will still choose nice nearby contour values, but now those nice nearby values will be near the desired vmin and vmax. This work was done during the SciPy 2013 matplotlib sprint.

…and then min or max out the input z values accordingly.
…utside the range of the data. In this case, the contour plot, if extend for the appropriate direction is also chosen, will appropriately reflect the desired range.
…ust call and not pop out the arguments. Turns out it is not necessary to use extend, this will just happen on its own.
@mdboom
Copy link
Member

mdboom commented Jun 29, 2013

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

@efiring
Copy link
Member

efiring commented Jun 29, 2013

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.

@dmcdougall
Copy link
Member

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.

@kthyng
Copy link
Contributor Author

kthyng commented Jun 29, 2013

from matplotlib import pyplot as plt
import numpy as np

# Get some data to use
xvec = np.linspace(0,10)
yvec = np.linspace(0,20)
X, Y = np.meshgrid(xvec,yvec)
Z = X**2 + Y**2
zmin, zmax = np.min(Z), np.max(Z)
print zmin, zmax

# Show plots
fig = plt.figure(figsize=(10,5))
fig.add_subplot(111)
ax1 = plt.subplot(1,2,1)
p = plt.pcolor(X,Y,Z,cmap='pink_r',vmax=700)
plt.title('pcolor plot')
plt.suptitle('With Fix')
cp = plt.colorbar()
ax2 = plt.subplot(1,2,2)
c = plt.contourf(X,Y,Z,cmap='pink_r', vmax=700)
plt.title('contourf plot')
cc = plt.colorbar()
plt.savefig('comp_with_fix.png',bbox_inches='tight')

comp_with_fix
comp_without_fix

@kthyng
Copy link
Contributor Author

kthyng commented Jun 29, 2013

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.

@pelson
Copy link
Member

pelson commented Jun 29, 2013

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.

Two image check tests were added to test_contour.py to make a contourf plot using input argument vmax, one with vmax greater than the maximum of the data set z values and one with vmax less than the maximum of the data set z values.
@kthyng
Copy link
Contributor Author

kthyng commented Jun 29, 2013

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.

@efiring
Copy link
Member

efiring commented Jun 29, 2013

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

@WeatherGod
Copy link
Member

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.

@kthyng
Copy link
Contributor Author

kthyng commented Jun 29, 2013

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

comp_with_fix
comp_without_fix

@efiring
Copy link
Member

efiring commented Jun 29, 2013

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

@kthyng
Copy link
Contributor Author

kthyng commented Jun 29, 2013

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.

@kthyng
Copy link
Contributor Author

kthyng commented Jun 30, 2013

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

@pelson
Copy link
Member

pelson commented Jan 14, 2014

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!

@pelson pelson closed this Jan 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants