Skip to content

BUG: contour plot levels #868

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

Merged
merged 7 commits into from
Jul 20, 2016
Merged

BUG: contour plot levels #868

merged 7 commits into from
Jul 20, 2016

Conversation

fmaussion
Copy link
Member

@fmaussion fmaussion commented Jun 3, 2016

This should fix #866

@fmaussion fmaussion force-pushed the contour branch 3 times, most recently from e8643aa to cb16fd9 Compare June 3, 2016 23:05
@fmaussion fmaussion changed the title fix: contour plot levels BUG: contour plot levels Jun 7, 2016
if levels is None:
levels = 7 # this is the matplotlib default
# A colorbar with one level is not possible with mpl
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a type check instead? e.g.,

if (isinstance(levels, int) and levels < 2) or len(levels) < 2:
    add_colorbar = False

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also change the default to add_colorbar=None, and only apply this logic if add_colorbar wasn't set directly.

@shoyer
Copy link
Member

shoyer commented Jul 19, 2016

I wonder -- would it maybe make more sense to simply not to add a colorbar by default? It's pretty easy to toggle if desired.

@fmaussion
Copy link
Member Author

@shoyer yes, this was my original idea too: contour plots usually don't need a colorbar. I changed the default to false for contour plots only, and removed the convoluted logic for when the user gives only one level: this is a matplotlib problem after all, and if the user sets levels=1 and add_colorbar=True it really isn't xarray's problem anymore.

I think it's better now, let me know what you think.

@@ -41,7 +41,7 @@ def _build_discrete_cmap(cmap, levels, extend, filled):

if not filled:
# non-filled contour plots
extend = 'neither'
extend = 'max'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I missed the rationale for this one -- why should this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual bug fix for #866 which motivated this PR ;-).

xarray wasn't plotting contours correctly before this change, see the following code:

import numpy as np
import matplotlib.pyplot as plt
import xarray as xr

x, y = np.meshgrid(np.arange(12), np.arange(12))
z = xr.DataArray(np.sqrt(x**2 + y**2))
z.plot.contour(levels=[2, 4, 6, 8], add_colorbar=False, colors='k')
plt.show()

figure_1

With this fix everything is fine now:

figure_1-1

Intuitively, it also makes sense: the last contour does mark all data beyond the last level, hence extend = 'max'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

@shoyer shoyer merged commit 884b247 into pydata:master Jul 20, 2016
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.

Drawing only one contour
2 participants