dask.array.asarray should handle case where xarray class is in top-level namespace#7335
Conversation
(Such as `xarray.DataArray`.)
jrbourbeau
left a comment
jrbourbeau
left a comment
There was a problem hiding this comment.
Thanks @tomwhite! What does type(a).__module__ look like in one of these problematic cases? Is this something we could easily test in dask/array/tests/test_xarray.py?
Do you think it would be safe to set the |
jrbourbeau
left a comment
jrbourbeau
left a comment
There was a problem hiding this comment.
Ah I see, so this issue is specifically related to using that intersphinx workaround, thanks for clarifying @tomwhite. This LGTM, I left a few small comments below mostly about adding some additional context around the added test
| xr.DataArray.__module__ = "xarray" | ||
| y = da.asarray(xr.DataArray([1, 2, 3.0])) | ||
| assert isinstance(y, da.Array) | ||
| assert type(y._meta).__name__ == "ndarray" |
There was a problem hiding this comment.
Just curious why this check is needed
There was a problem hiding this comment.
This is the assert that fails without the fix.
dask/array/tests/test_xarray.py
Outdated
| assert_eq(y, y) | ||
|
|
||
|
|
||
| def test_asarray_recognises_xarray_data(): |
There was a problem hiding this comment.
Could you add a comment here which points out that this test ensures we support users who are using the intersphinx workaround in pydata/xarray#4279
|
Also the test failures look to be unrelated to the changes here (I suspect it's due to a new |
|
FWIW I'm not sure if you're running into it, but there is similar looking module checking code over in Xarray itself |
Co-authored-by: James Bourbeau <jrbourbeau@users.noreply.github.com>
2 participants
In some cases (e.g. pydata/xarray#4279), xarray's
DataArrayclass is in thexarraymodule, rather than the usualxarray.core.dataarraymodule. This causesdask.array.asarrayto fail to wrap the array correctly, since it checks for a module name starting withxarray.(note the dot). This PR handles both cases.black dask/flake8 dask