Use to_numpy in time decoding
#10081
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcherian
force-pushed
the
duck-datetime-decoding
branch
from
c911894 to
f69ba29
Compare
dcherian
commented
Feb 27, 2025
| @@ -102,6 +102,10 @@ def to_numpy( | |||
| from xarray.core.indexing import ExplicitlyIndexed | |||
| from xarray.namedarray.parallelcompat import get_chunked_array_type | |||
|
|
|||
| if hasattr(data, "to_numpy"): | |||
There was a problem hiding this comment.
this is the ugly bit. It's only to test that we are using the to_numpy method instead of np.asarray which would fail for cupy.
There was a problem hiding this comment.
@keewis does this seem OK to you?
spencerkclark
approved these changes
Mar 1, 2025
There was a problem hiding this comment.
Nice, thanks @dcherian—not sure if you were looking for a review yet, but these changes look reasonable to me.
|
Thanks for the review @spencerkclark This just makes explicitly convert to numpy. Fully migrating to using duck arrays was more invasive and would conflict with existing PRs, so let's do that later. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the smallest set of changes I could find to make our time decoding handle duck arrays. Really we just explicitly convert to numpy arrays using
to_numpy. Note that the cftime code path uses Cython, and so, it requires numpyThe second (now-reverted) commit makes more invasive changes to avoid the numpy conversion if we can. I think it's best saved for a different PR.
cc @TomAugspurger
whats-new.rstapi.rst