add a CI that tests xarray with all optional dependencies but dask

[keewis]

Was using the py38 environment the correct choice? I'm also not sure if adding a new CI was as simple as adding two lines, but that's something I will see once the CI runs 😁

Edit: dask, dask-core and distributed get pulled in by other packages, so it seems there is a bit more investigation necessary.

• Closes all-dependencies-but-dask CI #3794
• Tests added
• Passes isort -rc . && black . && mypy . && flake8
• Fully documented, including whats-new.rst for all changes and api.rst for new API

[max-sixty]

Doesn't py3.6-bare-minimum cover this? Or you're thinking something that has more optional dependencies but not dask?

[keewis]

yes, exactly. There were a few issues we did not detect because there are code paths that are only chosen if dask is not available at all. See #3794 for a list of a few of these issues.

[keewis]

seems iris was the culprit that pulled in dask so I removed it.

I also renamed the CI to py38-all-but-dask but I'm not really confident in my naming abilities. If someone has a better idea I'd be happy to use that instead.

[max-sixty]

Ha, the naming is good!

[max-sixty]

This may be related: #1923

[keewis]

it is related, but this PR has a different set of failures: there are a lot of zarr related failures, two rasterio and one pseudonetcdf failure and also a sparse chunking failure. The sparse test and one of the rasterio tests obviously need to be decorated with requires_dask, but I don't know about the others.

[keewis]

the non-serializable lock from the failing rasterio test is from

xarray/xarray/backends/locks.py

Lines 6 to 10 in 280a14f

	try:
	from dask.utils import SerializableLock
	except ImportError:
	# no need to worry about serializing the lock
	SerializableLock = threading.Lock

which means the assumption that if we don't use dask we don't need to worry about serialization is incorrect (or the rasterio code uses the serializable lock in unintended ways)?

Also, the chunks parameter of open_zarr defaults to "auto", meaning it tries to chunk by default, even if dask is not available. Should we check the availability of dask and overwrite chunks="auto" with chunks=None if we can't import dask? This would reduce the number of failing zarr tests from 98 to 2. Edit: see fa37ee6

[keewis]

a bit more progress:

xarray/xarray/backends/zarr.py

Lines 58 to 60 in fa37ee6

	return array.vindex[
	indexing._arrayize_vectorized_indexer(key.tuple, self.shape).tuple
	]

_arrayize_vectorized_indexer accepts an indexer, so we should pass key directly.

This exposes a different error: plain zarr does not seem to support negative step sizes (zarr+dask seem fine with it, though).

[keewis]

it seems the slices get transformed differently depending on the data: for numpy arrays, slice(-1, 1, -1) is transformed into slice(9, 1, -1) where for dask arrays slice(-1, 1, -1) becomes something without a negative step (not sure how that looks, though).

I'd xfail these tests since I think they're bugs and open issues for them.

[keewis]

I don't think I can figure out how to fix the remaining three tests. Is it okay to have the fixes for open_zarr and ZarrArrayWrapper.__getitem__ in this PR?

If it is, this should be ready for review & merge.

[max-sixty]

Perfect, LGTM! Thank you @keewis

[dcherian]

Thanks @keewis. Clearly this is a useful test config to add!