Fix/tensor indexing bug #936#941
Merged
sandorkertesz merged 6 commits intorelease/1.0.0rc0from Mar 23, 2026
Merged
Conversation
…a.to_numpy() reverted from outer indexing to the classical numpy indexing (which might include advanced indexing which requires broadcastable indexers). Code cleaning
Collaborator
|
@pawel-wolff, thank you for this great improvement. |
This file contains hidden or 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
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.
Description
This PR fixes the issue #936.
The bug was caused by the recent upgrade (#932) of the earthkit tensor indexing support from
BASICtoOUTER(see https://docs.xarray.dev/en/stable/internals/how-to-add-new-backend.html#indexing-examples).With the
OUTERindexing support, xarray can ask the earthkit backend to sub-select a tensor axis using:The last kind of indexer turned out to be problematic. More specifically, if a field part
vof a tensor is a 2D lat-lon array and one wants to subselect it using a pair of indexers of the third kind (e.g.index=([9, 10, 11], [0, 1]), see:earthkit-data/tests/xr_engine/test_xr_engine_core.py
Line 226 in 8442708
v[index](earthkit-data/src/earthkit/data/core/field.py
Line 488 in 5d2b2af
does not work, because it applies the so-called "advanced indexing" (https://docs.xarray.dev/en/stable/internals/how-to-add-new-backend.html#indexing-examples).
In order to solve this problem, an outer indexing is applied explicitly to sub-select a field in the following methods:
Field.to_numpy():earthkit-data/src/earthkit/data/core/field.py
Line 488 in 8442708
Field.to_arrray():earthkit-data/src/earthkit/data/core/field.py
Line 534 in 8442708
Field.data():earthkit-data/src/earthkit/data/core/field.py
Line 618 in 8442708
Note that in the case of similar methods:
the indexing for a field is still
v[index], so the usual numpy-style indexing applies (which might be an advanced indexing, depending onindex).Several tests has been added or extended.
Contributor Declaration
By opening this pull request, I affirm the following: