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

(feat): support ellipsis indexing #1729

Merged
merged 25 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d2b859d
(feat): support ellipsis indexing
ilan-gold Oct 22, 2024
df109f6
(chore): release note
ilan-gold Oct 22, 2024
db86a20
(chore): add tests for all axis combinations
ilan-gold Oct 22, 2024
5822ed5
(fix): index typing
ilan-gold Oct 22, 2024
2a6c837
(feat): add support for 3d ellipsis/error on >1 ellipsis
ilan-gold Oct 24, 2024
87a2dbf
Merge branch 'main' into ig/ellipsis_indexing
ilan-gold Oct 24, 2024
c69aa7e
(fix): `unpack_index` type
ilan-gold Oct 24, 2024
eb36543
Merge branch 'ig/ellipsis_indexing' of github.com:scverse/anndata int…
ilan-gold Oct 24, 2024
174596a
(fix): ignore ellipsis in docs
ilan-gold Oct 24, 2024
f018328
(fix): import
ilan-gold Oct 24, 2024
f580cad
Update docs/conf.py
ilan-gold Oct 25, 2024
e994076
(chore): `is` instead of `isinstance` checks
ilan-gold Oct 25, 2024
7cfbb57
Merge branch 'ig/ellipsis_indexing' of github.com:scverse/anndata int…
ilan-gold Oct 25, 2024
f0b7e7e
(fix): try making `unpack_index` more general-purpose
ilan-gold Oct 25, 2024
a06675a
Apply suggestions from code review
ilan-gold Oct 25, 2024
ff5eb93
fix test_backed_ellipsis_indexing
flying-sheep Oct 25, 2024
52ea6d3
(refactor): simplify 3-elem index handling
ilan-gold Oct 25, 2024
95d9d37
(refactor): simplify tests
ilan-gold Oct 25, 2024
57c479c
merge
ilan-gold Oct 25, 2024
defbf37
(fix): use correct base-comparison
ilan-gold Oct 25, 2024
8a08e2b
Update src/anndata/_core/index.py
ilan-gold Oct 25, 2024
916cd3d
(fix): use `id` per param
ilan-gold Oct 25, 2024
8edffaf
Merge branch 'ig/ellipsis_indexing' of github.com:scverse/anndata int…
ilan-gold Oct 25, 2024
6aff0f2
(chore): split ellipsis index
ilan-gold Oct 25, 2024
37fd1e0
fix types
flying-sheep Oct 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 25 additions & 16 deletions src/anndata/_core/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from collections.abc import Iterable, Sequence
from functools import singledispatch
from itertools import repeat
from types import EllipsisType
from typing import TYPE_CHECKING

import h5py
Expand All @@ -14,7 +13,7 @@
from ..compat import AwkArray, DaskArray, SpArray

if TYPE_CHECKING:
from ..compat import Index, IndexRest
from ..compat import Index, Index1D


def _normalize_indices(
Expand All @@ -30,12 +29,10 @@
if len(index) > 2:
# only one remaining ellipsis
num_ellipsis = sum(i is Ellipsis for i in index)
if num_ellipsis == 1:
index = tuple(i for i in index if i is not Ellipsis)
elif num_ellipsis > 1:
if num_ellipsis > 1:
raise IndexError("an index can only have a single ellipsis ('...')")
if len(index) > 2:
if num_ellipsis == 0:
raise ValueError("AnnData can only be sliced in rows and columns.")

Check warning on line 35 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L35

Added line #L35 was not covered by tests
flying-sheep marked this conversation as resolved.
Show resolved Hide resolved
# deal with pd.Series
# TODO: The series should probably be aligned first
if isinstance(index[1], pd.Series):
Expand All @@ -55,8 +52,7 @@
| str
| Sequence[bool | int | np.integer]
| np.ndarray
| pd.Index
| EllipsisType,
| pd.Index,
index: pd.Index,
) -> slice | int | np.ndarray: # ndarray of int or bool
if not isinstance(index, pd.RangeIndex):
Expand All @@ -81,8 +77,6 @@
return slice(start, stop, step)
elif isinstance(indexer, np.integer | int):
return indexer
elif isinstance(indexer, EllipsisType):
return slice(None)
elif isinstance(indexer, str):
return index.get_loc(indexer) # int
elif isinstance(
Expand Down Expand Up @@ -118,7 +112,7 @@
"are not valid obs/ var names or indices."
)
return positions # np.ndarray[int]
raise IndexError(f"Unknown indexer {indexer!r} of type {type(indexer)}")

Check warning on line 115 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L115

Added line #L115 was not covered by tests


def _fix_slice_bounds(s: slice, length: int) -> slice:
Expand All @@ -140,15 +134,30 @@
return slice(start, stop, step)


def unpack_index(index: Index) -> tuple[IndexRest, IndexRest]:
def unpack_index(index: Index) -> tuple[Index1D, Index1D]:
if not isinstance(index, tuple):
if index is Ellipsis:
index = slice(None)
return index, slice(None)
elif len(index) == 2:
num_ellipsis = sum(i is Ellipsis for i in index)
if num_ellipsis > 1:
raise IndexError("an index can only have a single ellipsis ('...')")

Check warning on line 144 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L144

Added line #L144 was not covered by tests
# If index has Ellipsis, filter it out (and if not, error)
if len(index) > 2:
if not num_ellipsis:
raise IndexError("Received a length 3 index without an ellipsis")

Check warning on line 148 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L148

Added line #L148 was not covered by tests
index = tuple(i for i in index if i is not Ellipsis)
return index
elif len(index) == 1:
return index[0], slice(None)
else:
raise IndexError("invalid number of indices")
# If index has Ellipsis, replace it with slice
if len(index) == 2:
index = tuple(slice(None) if i is Ellipsis else i for i in index)
return index
if len(index) == 1:
index = index[0]
if index is Ellipsis:
index = slice(None)

Check warning on line 158 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L158

Added line #L158 was not covered by tests
return index, slice(None)
raise IndexError("invalid number of indices")

Check warning on line 160 in src/anndata/_core/index.py

View check run for this annotation

Codecov / codecov/patch

src/anndata/_core/index.py#L160

Added line #L160 was not covered by tests


@singledispatch
Expand Down
2 changes: 2 additions & 0 deletions src/anndata/compat/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class Empty:
IndexRest
| tuple[Index1D, IndexRest]
| tuple[IndexRest, Index1D]
| tuple[Index1D, Index1D, EllipsisType]
| tuple[EllipsisType, Index1D, Index1D]
ilan-gold marked this conversation as resolved.
Show resolved Hide resolved
| scipy.sparse.spmatrix
| SpArray
)
Expand Down
22 changes: 22 additions & 0 deletions tests/test_backed_sparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,28 @@ def test_backed_indexing(
assert_equal(csr_mem[:, var_idx].X, dense_disk[:, var_idx].X)


@pytest.mark.parametrize(
"indexing_func",
[
lambda x: (Ellipsis, slice(0, 10)),
lambda x: (slice(0, 10), Ellipsis),
lambda x: (slice(0, 10), slice(0, 10), Ellipsis),
lambda x: (Ellipsis, slice(0, 10), slice(0, 10)),
ilan-gold marked this conversation as resolved.
Show resolved Hide resolved
],
ids=["obs-ellipsis", "var-ellipsis", "obs-var-ellipsis", "ellipsis-obs-var"],
)
def test_backed_ellipsis_indexing(
ondisk_equivalent_adata: tuple[AnnData, AnnData, AnnData, AnnData],
indexing_func,
):
csr_mem, csr_disk, csc_disk, _ = ondisk_equivalent_adata

index = indexing_func(csr_mem)

assert_equal(csr_mem.X[index], csr_disk.X[index])
assert_equal(csr_disk.X[index], csc_disk.X[index])


def make_randomized_mask(size: int) -> np.ndarray:
randomized_mask = np.zeros(size, dtype=bool)
inds = np.random.choice(size, 20, replace=False)
Expand Down