issue_comments

I think we could bump the version even further (at least to 2.8, maybe also 2.9) so that should probably be a different PR

unless there are any objections (@shoyer?) I'm going to merge this tomorrow.

gentle ping, @shoyer

anyway, with this the tests finally pass :tada: so this should be ready for review and merging.

then it seems that this PR removes this intentional restriction. I'm not sure it is still needed, though: we don't use np.testing.assert_allclose internally because we want to provide our own error messages, and the actual comparison is done with https://github.com/pydata/xarray/blob/e5cc19cd8f8a69e0743f230f5bf51b7778a0ff96/xarray/core/duck_array_ops.py#L204 which should allow dispatching

hmm... using arr.compute() fails while np.array(arr) works. For now, I'm converting all dask arrays to numpy if the dask version is not high enough (I can't do that only for bool_ arrays since they have been converted to float somewhere).

@shoyer: I'm confused, I thought that xr.testing.assert_allclose explicitly supported duck arrays (it calls duck_array_ops.allclose_or_equiv). TBC, what I was talking about was these lines: https://github.com/pydata/xarray/blob/e5cc19cd8f8a69e0743f230f5bf51b7778a0ff96/xarray/testing.py#L124 https://github.com/pydata/xarray/blob/e5cc19cd8f8a69e0743f230f5bf51b7778a0ff96/xarray/testing.py#L132-L135 where the .values always convert to numpy

let's see if that works

I needed to use assert_allclose in the pint tests in #3975, so I modified it to use .data instead of .values and now the exact same tests are failing. So it seems that assert_allclose never worked with dask but always converted to numpy. Would it make sense to investigate further, or is it better to just wait until we can merge this?

Unfortunately, simply replacing assert np.allclose(...) with np.testing.assert_allclose(...) does not fix this. Also, I undid the swapping of arguments to allclose_or_equiv which makes the min-all-deps CI fail, too. Does anyone know why that works?

Would it make sense to wait until we can bump the dask version to 2.9?

@pydata/xarray, this is really close but I'm not familiar enough with dask to get it to work with dask<2.9.1.

Once we get the py36-min-all-deps and py36-min-nep18 CI to pass, this should be ready for a final review and merging.

Edit: there seem to be a few failing tests related to pint (I will fix those) but the tests in question are in test_duck_array_ops.py and only fail with py36-min-nep18.

The issue is with how dask<2.9.1 handles dtypes on compute (in nanvar?) when the data is an array with dtype object filled with False and some missing values represented by np.nan.

I really lack experience with dask, though, so I'm clueless as to what to do to fix that (besides calling compute before passing to allclose_or_equiv) and would appreciate help with this.

unfortunately, we need dask>=2.9.1 for that. I could try debugging a bit more to find out exactly why it fails (or someone helps me with that?), but that would take me a bit more time than just skipping / xfailing those tests if on dask<2.9.1.

to me the failing tests on the min-all-deps and min-nep18 CI seem like a dask issue (in var?) that has been resolved in later versions: if we compute actual before passing it to assert_allclose the tests don't fail. How should I resolve this?

Also, where should I put the whats-new.rst entry?

now only the min-all-deps and min-nep18 CI fail. I suspect that is due to the pinned dependencies.

the failures should be a bug in pint and hopefully fixed by hgrecco/pint#1044