issue_comments

OK, so thanks to https://github.com/pydata/xarray/pull/1191 the required changes for the tests to pass in mpl v2 are minimal: - deprecation of get_axis_bgcolor - test_num_ticks also changed due to changes in MaxNLocator, but this is not a big deal

This is ready for review / merge

I've never been completely satisfied with what we came up with for the vmin/vmax/levels logic. Its been the source of a few bugs and confusion (#1015). The feature set we were going for is mostly covered (discrete colorbar, flexibility in specification of levels, etc.) but we may want to revisit the approach.

@shoyer

if neither are set, then perhaps we should differ to whatever matplotlib defaults to

AFAIK, there is no logic for nlevels=int in mpl. Furthermore, mpl uses vmin and vmax to clip the data (not to decide upon the levels), which is undesirable for us.

However, the default mpl behavior when no levels are set is to call the private method _autolev which has a logic based on MaxNLocator too.

Long story short: I'll start another PR to address this problem specifically and then go back to MPL2 later on.

@efiring thanks for jumping in! I understand and I see that xarray is kind of misusing this tool, we'll change this in this PR.

In the test above MaxNLocator returns N+2 ticks though.

My feeling is that vmin and vmax should always be exactly at the specified locations, but the location of the ticks in between is a little more flexible. If both vmin/vmax are set, then using linspace seems obvious, and if neither are set, then perhaps we should differ to whatever matplotlib defaults to. If only one is set, the solution is less clear. Maybe one limit be chosen exactly and the other in a "nice" way?

@QuLogic : thanks, and sorry for the noise.

@shoyer : should we use linspace only if vmax and/or vmin are set explicitly, or always? MaxNLocator definitely makes "nicer" levels, but as stated by the name it might choose less than N levels if needed...

On a side note, I already encountered this problem with my students before: what most users will do in this case is to set levels=np.linspace(foo, bar) themselves

@fmaussion as you note, even if this is a matplotlib bug, it seems like this was not doing quite the desired thing in xarray even with matplotlib 1.5 -- we really should be using np.linspace for this, not MaxNLocator.

The Matplotlib 2.0 release notes mention that the algorithm for the number of ticks in MaxNLocator has been changed but this seems at odds with the documented behavior for specifying a minimum number of intervals.

I've found this ticket, but it's not clear to me if the problem is solved or not. I'll have to test against mpl2 master I guess...

@QuLogic do you maybe have a moment to have a look at my comment above?

I think this is a bug

Yes, I should've looked into this in more detail. So the problem is here. MaxNLocator.tick_values(vmin, vmax) does not guarantee to land on vmin or vmax exactly, which I think is confusing. It should be either changed or documented in xarray. The change could look like: if vmin and vmax are set explicitely, than we use linspace to build the levels.

Furthermore, MaxNLocator is no longer guaranteed to return a max of N levels in MPLv2, as shown by this simple test:

python import matplotlib as mpl ticker = mpl.ticker.MaxNLocator(7) levels = ticker.tick_values(4, 8) print('mpl: {}, nlevels: {}, levels: {}'.format(mpl.__version__, len(levels), levels))

Which prints, depending on mpl version:

mpl: 1.5.3, nlevels: 7, levels: [ 3.2 4. 4.8 5.6 6.4 7.2 8. ]

mpl: 2.0.0rc2, nlevels: 9, levels: [ 3.6 4.2 4.8 5.4 6. 6.6 7.2 7.8 8.4]

@fmaussion I think this is a bug, either with xarray plotting or matplotlib 2. If you supply vmin and vmax explicitly, shouldn't those be the minimum and maximum tick labels?

Also, shouldn't you get the requested number of levels? In matplotlib 2.0, I see 9 levels, not 7.

This fixes the failing tests on MPL2. Two of the tests were failing because of how mpl chooses to do things, which makes me think that the tests are not very robust. My fixes are also rather simple...

For example the test_discrete_colormap_int_levels test, reproduced here:

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

x = np.arange(start=0, stop=10, step=2) y = np.arange(start=9, stop=-7, step=-3) xy = np.dstack(np.meshgrid(x, y)) distance = np.linalg.norm(xy, axis=2) darray = xr.DataArray(distance, list(zip(('y', 'x'), (y, x))))

darray.plot(levels=7, vmin=4, vmax=8, extend='both') plt.title('mpl {}'.format(matplotlib.version)) ```

produces:

@fmaussion Great. No rush.

@jhamman I planned to do so but I won't come to this in the next couple of days, so if its urgent and you want to do it go ahead! They should be easy fixes

@fmaussion - do you want to take point on resolving these failing tests?

... three failing tests. Not too bad!