NEP-35 duck array update by jrbourbeau · Pull Request #7321 · dask/dask

jrbourbeau · 2021-03-04T17:02:14Z

This tests out adding an __array_function__ condition to our is_arraylike check (xref #7263 (comment)). Let's see if CI passes

cc @pentschev @jakirkham

Closes Unstable dask causes unexpected compute results from xarray #7263
Tests added / passed
Passes black dask / flake8 dask

jakirkham · 2021-03-04T17:41:39Z

cc @shoyer

jrbourbeau · 2021-03-04T18:25:49Z

Just to summarize the test failures we're seeing:

The mindeps build is failing because we support versions of NumPy that predate the addition of __array_function__ which causes is_arraylike to return False on things like NumPy arrays
There is one test, dask/dataframe/tests/test_dataframe.py::test_dask_dataframe_holds_scipy_sparse_containers which is failing because scipy.sparse arrays don't implement __array_function__. FWIW I tracked this test back to Test that dask collections can hold scipy.sparse arrays #3738 where it looks like we don't fully support scipy.sparse arrays inside Dask arrays, but it is useful to partially support for some Dask-ML work (hand chunks off to dask.delayed or da.map_blocks workflows)

jakirkham · 2021-03-04T18:27:55Z

We also support CuPy sparse matrices for similar reasons, which likely have similar issues

pentschev · 2021-03-04T18:34:03Z

Thanks @jrbourbeau , you beat me to it on working this PR. 😄

The mindeps build is failing because we support versions of NumPy that predate the addition of __array_function__ which causes is_arraylike to return False on things like NumPy arrays

For this, we'll also want to check for

dask/dask/array/utils.py

Line 524 in 5273e73

IS_NEP18_ACTIVE = _is_nep18_active()

.

There is one test, dask/dataframe/tests/test_dataframe.py::test_dask_dataframe_holds_scipy_sparse_containers which is failing because scipy.sparse arrays don't implement __array_function__. FWIW I tracked this test back to Test that dask collections can hold scipy.sparse arrays #3738 where it looks like we don't fully support scipy.sparse arrays inside Dask arrays, but it is useful to partially support for some Dask-ML work (hand chunks off to dask.delayed or da.map_blocks workflows)

Seems like we'll need to special-case this as well in is_arraylike, it doesn't look like we have a much better alternative.

dask/utils.py

jakirkham

Some suggestions for CuPy sparse matrices

dask/utils.py

jakirkham · 2021-03-04T23:47:39Z

dask/utils.py

    """
    from .base import is_dask_collection

+    is_duck_array = hasattr(x, "__array_function__") or hasattr(x, "__array_ufunc__")


@pentschev do you have thoughts on this formulation?

xref-ing @pentschev's earlier comment #7321 (comment)

The duck arrays I'm aware of (i.e. NumPy, sparse, cupy, pint, unyt) all will result in is_duck_array being True. That said, I can't be 100% certain that this covers all array-like libraries that are used with Dask today. Perhaps this is a good opportunity for us to specify the conditions for which an array library will work with Dask Array. Xarray has a nice documentation section on this http://xarray.pydata.org/en/stable/internals.html#internals-duck-arrays topic

jrbourbeau · 2021-03-05T03:50:03Z

If there are no further comments I'll propose we include this in the release tomorrow. @jakirkham @pentschev could I ask one of you to run the cupy tests with the changes here to ensure they pass?

pentschev · 2021-03-05T07:08:20Z

If there are no further comments I'll propose we include this in the release tomorrow. @jakirkham @pentschev could I ask one of you to run the cupy tests with the changes here to ensure they pass?

I agree with changes here and tested it locally, the tests pass with the exception of those I mentioned in #7324 . I also applied the patch to a start just after #6738 and by doing so all tests would pass, so from the CuPy side I thing we're good.

Thanks @jrbourbeau for working on this! 😄

shoyer · 2021-03-05T08:11:58Z

Thanks for working on this -- yes, this is important to keep Xarray working with dask :)

My only thought is that the name is_arraylike() is not quite appropriate, given that NumPy uses "array like" for a different meaning. I would suggest using the name is_duck_array() instead.

pentschev · 2021-03-05T14:25:42Z

My only thought is that the name is_arraylike() is not quite appropriate, given that NumPy uses "array like" for a different meaning. I would suggest using the name is_duck_array() instead.

I do not oppose doing that, and semantically agree with it. I'm only wondering if renaming that may cause confusion with people, we also have other utility function such as is_dataframe_like and is_series_like with similar meaning, but perhaps renaming and adding that information to docstrings would suffice?

jakirkham · 2021-03-05T15:28:43Z

Given this is a bug fix we are trying to get in before a release and I don’t think this function is intended for usage outside Dask, maybe we can punt on function renaming and track that in a new issue?

jrbourbeau · 2021-03-05T16:46:54Z

maybe we can punt on function renaming and track that in a new issue?

+1. @shoyer thanks for bringing this terminology issue up -- I agree there's a mismatch between the is_arraylike method and the term "array-like". Let's discuss in a follow-up issue

jrbourbeau · 2021-03-05T16:48:50Z

Thanks all for reviewing! Your feedback was very useful

jakirkham · 2021-03-05T17:02:09Z

Thanks James! 😄

shoyer · 2021-03-05T17:53:22Z

Thanks! I agree, this might be clarified well just by updating the docstring for this helper function.

…

On Fri, Mar 5, 2021 at 9:02 AM jakirkham ***@***.***> wrote: Thanks James! 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7321 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJJFVS3DDSKGHAK7EKYRY3TCEFCFANCNFSM4YTT3AEQ> .

jakirkham · 2021-03-05T19:00:18Z

Filed issue ( #7327 ) on that point. Also noted updating the docstring. Please feel free to add anything else there 🙂

* upstream/master: (43 commits) bump version to 2021.03.0 Bump minimum version of distributed (dask#7328) Fix `percentiles_summary` with `dask_cudf` (dask#7325) Temporarily revert recent Array.__setitem__ updates (dask#7326) Blockwise.clone (dask#7312) NEP-35 duck array update (dask#7321) Don't allow setting `.name` for array (dask#7222) Use nearest interpolation for creating percentiles of integer input (dask#7305) Test `exp` with CuPy arrays (dask#7322) Check that computed chunks have right size and dtype (dask#7277) pytest.mark.flaky (dask#7319) Contributing docs: add note to pull the latest git tags before pip installing Dask (dask#7308) Support for Python 3.9 (dask#7289) Add broadcast-based merge implementation (dask#7143) Add split_every to graph_manipulation (dask#7282) Typo in optimize docs (dask#7306) dask.graph_manipulation support for xarray.Dataset (dask#7276) Add plot width and height support for Bokeh 2.3.0 (dask#7297) Add numpy functions tri, triu_indices, triu_indices_from, tril_indices, tril_indices_from (dask#6997) Remove "cleanup" task in dataframe on-disk shuffle. The partd directory (dask#7260) ...

Add __array_function__ condition to is_arraylike

3c1971a

jrbourbeau added 2 commits March 4, 2021 14:59

Add NEP-18 check

19144a1

Special case scipy.sparse

bb3c0f1

jrbourbeau commented Mar 4, 2021

View reviewed changes

dask/utils.py Outdated Show resolved Hide resolved

jakirkham reviewed Mar 4, 2021

View reviewed changes

dask/utils.py Show resolved Hide resolved

dask/utils.py Outdated Show resolved Hide resolved

jrbourbeau added 2 commits March 4, 2021 17:17

Specify cupyx.scipy.sparse arrays are also special cased

f6cf004

Update duck array condition

6ba5eab

jakirkham reviewed Mar 4, 2021

View reviewed changes

jrbourbeau changed the title ~~[WIP] NEP-35 update~~ NEP-35 duck array update Mar 5, 2021

pentschev approved these changes Mar 5, 2021

View reviewed changes

jakirkham approved these changes Mar 5, 2021

View reviewed changes

jrbourbeau merged commit fa6203c into dask:master Mar 5, 2021

jrbourbeau deleted the nep-35-fixup branch March 5, 2021 16:48

jakirkham mentioned this pull request Mar 5, 2021

Unstable dask causes unexpected compute results from xarray #7263

Closed

jakirkham mentioned this pull request Mar 5, 2021

Rename is_arraylike to is_duck_array #7327

Closed

dependabot bot mentioned this pull request Mar 8, 2021

Update dask requirement from ^2020.12.0 to ^2021.3.0 Kwonil-Kim/kkpy#8

Closed

jni mentioned this pull request Mar 11, 2021

test_thresholding fails with last dask version (2021.3.0) scikit-image/scikit-image#5266

Closed

dependabot bot mentioned this pull request Mar 12, 2021

Bump dask[bag] from 2.24.0 to 2021.3.0 admdev8/pyglotaran#52

Closed

Uh oh!

Conversation

jrbourbeau commented Mar 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakirkham commented Mar 4, 2021

Uh oh!

jrbourbeau commented Mar 4, 2021

Uh oh!

jakirkham commented Mar 4, 2021

Uh oh!

pentschev commented Mar 4, 2021

Uh oh!

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jakirkham Mar 4, 2021

Choose a reason for hiding this comment

Uh oh!

jrbourbeau Mar 4, 2021

Choose a reason for hiding this comment

Uh oh!

jrbourbeau Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

jrbourbeau commented Mar 5, 2021

Uh oh!

pentschev commented Mar 5, 2021

Uh oh!

shoyer commented Mar 5, 2021

Uh oh!

pentschev commented Mar 5, 2021

Uh oh!

jakirkham commented Mar 5, 2021

Uh oh!

jrbourbeau commented Mar 5, 2021

Uh oh!

jrbourbeau commented Mar 5, 2021

Uh oh!

jakirkham commented Mar 5, 2021

Uh oh!

shoyer commented Mar 5, 2021 via email

Uh oh!

jakirkham commented Mar 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jrbourbeau commented Mar 4, 2021 •

edited

Loading