issue_comments

This is a one-line change (e378dbc0553dc84c953550fa678028c6299a330f). Do we need tests?

@dcherian Do you mean merge #3274 here (not sure what's the best workflow)? Also am I correct this will break compatibility with matplotlib<3.1? I think I'm done otherwise.

@dcherian could you please briefly explain the difference between class TestPlot and class TestPlot1D or point me to docs on that?

@dcherian Thanks for pointing the problematic code (in #3571). This commit (b8f082b) solves the issue, I think. Or do you think it will be cleaner with a new util _resolve_intervals_1dplot (similar to the existing _resolve_intervals_2dplot)?

@TomNicholas I've had a closer look at the code. Nested lists of file paths are processed by:

https://github.com/pydata/xarray/blob/f2b2f9f62ea0f1020262a7ff563bfe74258ffaa1/xarray/backends/api.py#L881-L882

Using the method defined in:

https://github.com/pydata/xarray/blob/f2b2f9f62ea0f1020262a7ff563bfe74258ffaa1/xarray/core/combine.py#L15-L46

In Python 3.7+ the list of paths is essentially flattened list e.g.:

```

import xarray as xr paths = [list('abc'), list('def')] list(xr.core.combine._infer_concat_order_from_positions(paths).values()) ['a', 'b', 'c', 'd', 'e', 'f'] `` Unfortunately the current code uses a dictionary which means that in Python 3.6- the order is not guaranteed preserved. This also implies that the current default0` is not well defined in case of nested lists.

https://github.com/pydata/xarray/blob/f2b2f9f62ea0f1020262a7ff563bfe74258ffaa1/xarray/backends/api.py#L900 https://github.com/pydata/xarray/blob/f2b2f9f62ea0f1020262a7ff563bfe74258ffaa1/xarray/backends/api.py#L964

On the other hands the ids are essentially ND indexes that could perhaps be used...

```

list(xr.core.combine._infer_concat_order_from_positions(paths).keys()) [(0, 0), (0, 1), (0, 2), (1, 0), (1, 1), (1, 2)] ```

Or should we just stick to file paths as you suggest? And leave the default as is (e.g. ambiguous for Python 3.6-)?

@TomNicholas Thanks for bringing the discussion live again! I'm not sure what happens in those cases, but I'm confident the default behaviour is unchanged, i.e. the attributes file is 0, whatever that 0 means (see my first commit).

If this is an issue I would suggest to discuss in a separate thread, as I think it is independent from my changes. On the other hand I am eager to keep the file number option because (1) attrs_file=-1 is the behaviour that I need (to ensure that history is always preserved) and (2) attrs_file=0 is the current behaviour (again, whatever that means for nested lists).

This will add a new kwarg in open_mfdataset. The current name is attrs_file, defaulting to 0 (current behaviour). Or? Suggestions welcome.

I think I'm done. Can someone look at it? The master_file keyword argument is borrowed from NetCDF (see #2382 and Unidata/netcdf4-python#835), although xarray's mechanism is independant.

The default is 0, which is consistent with current xarray behaviour. When concatenating model output in time it would be more logical to use -1, i.e. preserve history of the last file.

@dcherian Thanks! This works for me.

I appreciate that my current code using the workaround plot(norm=norm, vmin=norm.vmin, ...) still works. To be fully consistent I suggest to replace the error message Cannot supply vmin and a norm with vmin with Cannot supply vmin and a norm with a different vmin (idem with vmax).

Edit: I just checked that matplotlib's imshow accepts imshow(norm=norm, vmin=vmin, vmax=vmax), with the user-provided vmin taking precedence over the norm. So I'm not sure that xarray need to raise an error here.

I was working on it but you were quicker, thanks!