Skip to content

Enable explicit use of key tuples (instead of *Indexer objects) in indexing adapters and explicitly indexed arrays - #8870

Merged
dcherian merged 27 commits into
pydata:backend-indexingfrom
andersy005:pass-key-tuple-to-indexing-adapters
May 3, 2024
Merged

Enable explicit use of key tuples (instead of *Indexer objects) in indexing adapters and explicitly indexed arrays#8870
dcherian merged 27 commits into
pydata:backend-indexingfrom
andersy005:pass-key-tuple-to-indexing-adapters

Conversation

@andersy005

@andersy005andersy005 commented Mar 23, 2024

Copy link
Copy Markdown
Member

@andersy005andersy005 changed the title Enable explicit use of key tuples (instead of *Indexer objects) for indexing in indexing adapters and explicitly indexed arraysEnable explicit use of key tuples (instead of *Indexer objects) in indexing adapters and explicitly indexed arraysMar 23, 2024
Illviljan
Illviljan reviewed Mar 26, 2024
View reviewed changes
Comment threadxarray/coding/strings.py Outdated
@andersy005andersy005 mentioned this pull request Mar 28, 2024
Merged
4 tasks
@andersy005
andersy005 changed the base branch from main to backend-indexingApril 10, 2024 00:07
andersy005and others added 11 commits April 9, 2024 17:07
@andersy005
Merge branch 'backend-indexing' into pass-key-tuple-to-indexing-adapters
c3c7cce
@andersy005
Merge branch 'backend-indexing' into pass-key-tuple-to-indexing-adapters
3a3d42f
@andersy005
Merge branch 'backend-indexing' into pass-key-tuple-to-indexing-adapters
07a14b8
@andersy005
Update indexing in StackedBytesArray
52c0f4c
@andersy005
use tuple indexing in test_backend_array_deprecation_warning
3c76fc5
@andersy005
Add support for CompatIndexedTuple in explicit indexing adapter
f062d0d
This commit updates the `explicit_indexing_adapter` function to accept both
`ExplicitIndexer` and the new `CompatIndexedTuple`. The `CompatIndexedTuple` is
designed to facilitate the transition towards using raw tuples by carrying
additional metadata about the indexing type (basic, vectorized, or outer).
@andersy005
remove unused code
fb17dc2
@andersy005
type hint fixes
5fa48eb
@andersy005
fix docstrings
cdd605f
@andersy005
fix tests
f35a72b
@andersy005
fix docstrings
e646bbf
@andersy005
andersy005 marked this pull request as ready for review April 30, 2024 12:10
@andersy005andersy005 mentioned this pull request Apr 30, 2024
Open
20 tasks
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/coding/strings.py Outdated
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/coding/variables.py Outdated
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/core/indexing.py Outdated
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/tests/test_backends.py
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
la.vindex[indexer].get_duck_array()
la.vindex[indexer.tuple].get_duck_array()

@dcheriandcherianApr 30, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
la.vindex[indexer.tuple].get_duck_array()
la[indexer].get_duck_array()

This is where we check for backcompat warnings in custom backends. Is the warning being raised when we pass .tuple to .vindex? It should not be..

@andersy005andersy005May 2, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

i intend to look into this later today

dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/tests/test_coding_strings.py Outdated
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/tests/test_dataset.py
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/core/indexing.py
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/core/indexing.py Outdated
andersy005and others added 2 commits April 30, 2024 14:13
@andersy005@dcherian
Apply suggestions from code review
2b273ab
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
@andersy005
update docstrings and pass tuples directly
237bbe2
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/core/indexing.py
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/core/indexing.py Outdated
key = BasicIndexer(
indexer.tuple if isinstance(indexer, ExplicitIndexer) else indexer
)
return type(self)(self.array, self._updated_key(key))

@dcheriandcherianApr 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's fix self._updated_key to not need Indexer objects

@andersy005andersy005May 2, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

added some initial updates with support for raw tuples as input. however, there's still use of indexer objects within the method itself. my attempts at completely removing indexer objects resulted in plenty of issues that i wasn't ready to address :). will look into this later today as well

dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/coding/strings.py Outdated
dcherian
dcherian reviewed Apr 30, 2024
View reviewed changes
Comment threadxarray/coding/variables.py Outdated
@dcherian
dcherian merged commit 245c3db into pydata:backend-indexingMay 3, 2024
@dcherian

dcherian commented May 3, 2024

Copy link
Copy Markdown
Contributor

Very nice work, Anderson!

We can address the backend warnings in the next PR

andersy005 added a commit that referenced this pull request May 10, 2024
@andersy005@dcherian
Enable explicit use of key tuples (instead of *Indexer objects) in in…
96ac4b7
…dexing adapters and explicitly indexed arrays (#8870)
* pass key tuple to indexing adapters and explicitly indexed arrays
* update indexing in StackedBytesArray
* Update indexing in StackedBytesArray
* Add _IndexerKey type to _typing.py
* Update indexing in StackedBytesArray
* use tuple indexing in test_backend_array_deprecation_warning
* Add support for CompatIndexedTuple in explicit indexing adapter
This commit updates the `explicit_indexing_adapter` function to accept both
`ExplicitIndexer` and the new `CompatIndexedTuple`. The `CompatIndexedTuple` is
designed to facilitate the transition towards using raw tuples by carrying
additional metadata about the indexing type (basic, vectorized, or outer).
* remove unused code
* type hint fixes
* fix docstrings
* fix tests
* fix docstrings
* Apply suggestions from code review
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
* update docstrings and pass tuples directly
* Some test cleanup
* update docstring
* use `BasicIndexer` instead of `CompatIndexedTuple`
* support explicit indexing with tuples
* fix mypy errors
* remove unused IndexerMaker
* Update LazilyIndexedArray._updated_key to support explicit indexing with tuples
---------
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
Co-authored-by: Deepak Cherian <deepak@cherian.net>
andersy005 added a commit to hmaarrfk/xarray that referenced this pull request May 10, 2024
@andersy005
Merge branch 'backend-indexing' into tuples_backend_indexing
d1edd15
* backend-indexing:
Trigger CI only if code files are modified. (pydata#9006)
Enable explicit use of key tuples (instead of *Indexer objects) in indexing adapters and explicitly indexed arrays (pydata#8870)
add `.oindex` and `.vindex` to `BackendArray` (pydata#8885)
temporary enable CI triggers on feature branch
Avoid auto creation of indexes in concat (pydata#8872)
Fix benchmark CI (pydata#9013)
Avoid extra read from disk when creating Pandas Index. (pydata#8893)
Add a benchmark to monitor performance for large dataset indexing (pydata#9012)
Zarr: Optimize `region="auto"` detection (pydata#8997)
Trigger CI only if code files are modified. (pydata#9006)
Fix for ruff 0.4.3 (pydata#9007)
Port negative frequency fix for `pandas.date_range` to `cftime_range` (pydata#8999)
Bump codecov/codecov-action from 4.3.0 to 4.3.1 in the actions group (pydata#9004)
Speed up localize (pydata#8536)
Simplify fast path (pydata#9001)
Add argument check_dims to assert_allclose to allow transposed inputs (pydata#5733) (pydata#8991)
Fix syntax error in test related to cupy (pydata#9000)
wavebyrd pushed a commit to wavebyrd/xarray that referenced this pull request Mar 13, 2026
@andersy005@dcherian
Enable explicit use of key tuples (instead of *Indexer objects) in in…
0dfea32
…dexing adapters and explicitly indexed arrays (pydata#8870)
* pass key tuple to indexing adapters and explicitly indexed arrays
* update indexing in StackedBytesArray
* Update indexing in StackedBytesArray
* Add _IndexerKey type to _typing.py
* Update indexing in StackedBytesArray
* use tuple indexing in test_backend_array_deprecation_warning
* Add support for CompatIndexedTuple in explicit indexing adapter
This commit updates the `explicit_indexing_adapter` function to accept both
`ExplicitIndexer` and the new `CompatIndexedTuple`. The `CompatIndexedTuple` is
designed to facilitate the transition towards using raw tuples by carrying
additional metadata about the indexing type (basic, vectorized, or outer).
* remove unused code
* type hint fixes
* fix docstrings
* fix tests
* fix docstrings
* Apply suggestions from code review
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
* update docstrings and pass tuples directly
* Some test cleanup
* update docstring
* use `BasicIndexer` instead of `CompatIndexedTuple`
* support explicit indexing with tuples
* fix mypy errors
* remove unused IndexerMaker
* Update LazilyIndexedArray._updated_key to support explicit indexing with tuples
---------
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
Co-authored-by: Deepak Cherian <deepak@cherian.net>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcheriandcheriandcherian left review comments
@IllviljanIllviljanIllviljan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andersy005@dcherian@Illviljan