issue_comments

@jhamman you should merge this once you and @clarkfitzg are happy. I'm about go offline for 6 days (backpacking in the Olympics).

@clarkfitzg any thoughts here?

These tests look great -- thanks!

Decided to try reorganizing things and got sucked in, so I ended up making a PR to your fork with some tweaks: https://github.com/jhamman/xray/pull/1

This assuredly still needs more tests -- mostly to verify that every plot types ends up with consistent levels.

Here's my demo notebook: https://gist.github.com/shoyer/e6fafbc672021600d9f3

and a teaser:

The main difference in functionality is in my PR is that vmin and vmax cannot be set independently of levels, if levels is provided as a list. That seemed a little too flexible, and this way we can get a consistent result across all four plot types. Are there any cases where you would want to set vmin/vmax to something other than the first and last levels?

Also, I entirely replaced the levels calculation in contour/contourf with our calculated levels instead of merely trying to replicate it. That ensures things are guaranteed to be exactly identical across all plot types.

Options for when levels is provided and extend == None:

I would opt for your solution 2. I think this basically means using levels[0] and levels[-1] if levels are provided explicitly instead of vmin/vmax (which are being ignored anyways).

Looking at the matplotlib source code, here's how they figure out levels in contour/contourf: https://github.com/matplotlib/matplotlib/blob/714d18788320325d0bff75184f62d472f67ceb91/lib/matplotlib/contour.py#L1143

It looks like they use MaxNLocator to do this: http://matplotlib.org/api/ticker_api.html#matplotlib.ticker.MaxNLocator

It should be pretty straightforward to use that here.

Some feedback: 1. matplotlib seems to have nice heuristics for picking level splits with contour plots. I wonder if it would be feasible to copy that logic here when setting our own discrete levels? Compare: ds.t2m[3].plot(levels=6) vs ds.t2m[3].plot.contourf() 2. The logic for inferring the extend keyword argument needs to be updated to handle levels: ds.t2m[3].plot(robust=True, levels=[200, 250, 260, 270, 280, 290, 350])

3.imshow, pcolormesh and contourf seem to deal with an explicit list of levels the exact same way. That's a relief. The plots look exactly the same. 4. As you noted, contourf currently crashes if you supply levels as an integer. This should be fixed, probably by converting levels into a 1d array / list if they are supplied as an integer before passing it into contourf. This would actually be more broadly useful, because currently there is no way to supply a non-default integer number of levels with contourf -- matplotlb appears to only expose the N argument when it is used positionally.