Skip to content

Conversation

@mrocklin
Copy link
Member

Fixes #4532

  • Tests added / passed
  • Passes flake8 dask

cc @mraspaud @shoyer if you have time to take a look at this I would appreciate it.

Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks like the right way to handle xarray objects in dask. (Short of a more generic numpy duck-typing protocol that we could use instead)

def test_xarray():
y = da.mean(xr.DataArray([1, 2, 3.0]))

assert_eq(y, y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this verify that the type of the result is a dask array? assert isinstance(y, da.Array) might be a more direct way to do this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I could imagine future behavior being that the result here is an Xarray thing rather than a Dask thing. I actually don't care too much about the output, as long as it computes and is self-consistent.

@jcrist
Copy link
Member

jcrist commented Apr 30, 2019

LGTM, merging.

@jcrist jcrist merged commit 5f69afe into dask:master Apr 30, 2019
@jcrist jcrist deleted the xarray-normalize branch April 30, 2019 19:32
jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Xarray handling regression in 1.1.2

3 participants