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

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

@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

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]

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:

@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

... three failing tests. Not too bad!