Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cupy in as_shared_dtype #4232

Merged
merged 7 commits into from
Jul 24, 2020

Conversation

jacobtomlinson
Copy link
Contributor

@jacobtomlinson jacobtomlinson commented Jul 16, 2020

This implements solution 2 for #4231.

cc @quasiben

@jacobtomlinson jacobtomlinson marked this pull request as draft July 16, 2020 16:52
Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

When I was working on adding pint support, I was also thinking about that (but I can't seem to find the exact issue). In the end, we implemented option , since it normally doesn't matter which unit 0 and nan have. However, this doesn't work with numpy.nanprod where _replace_nan is called with 1. In the end I didn't try to implement this because I didn't want to add code that special-cases pint (that is why e.g. DataArray.prod is not supported with pint arrays, yet).

With you also running into this issue it might make sense to generalize this: maybe get as_shared_dtype to cast to a common type instead of always casting to numpy?

xarray/core/duck_array_ops.py Show resolved Hide resolved
xarray/core/pycompat.py Outdated Show resolved Hide resolved
xarray/core/variable.py Show resolved Hide resolved
@jacobtomlinson jacobtomlinson marked this pull request as ready for review July 23, 2020 16:09
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

just one minor comment

xarray/tests/test_cupy.py Outdated Show resolved Hide resolved
jacobtomlinson and others added 4 commits July 24, 2020 15:11
* upstream/master:
  Added xarrays-spatial and updated geoviews link (pydata#4262)
  update docs to point to xarray-contrib and xarray-tutorial (pydata#4252)
  Add release summary, some touch-ups (pydata#4217)
  CFTimeIndex calendar in repr (pydata#4092)
  fix the RTD timeouts (pydata#4254)
  update isort CI and pre-commit hook (pydata#4204)
@dcherian
Copy link
Contributor

Failed test is #4265

I'm merging so things can keep moving.

@dcherian dcherian merged commit b1c7e31 into pydata:master Jul 24, 2020
@jacobtomlinson jacobtomlinson deleted the as_shared_dtype_cupy branch July 27, 2020 10:32
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.

as_shared_dtype coerces scalars into numpy regardless of other array types
5 participants