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.

Sign up for GitHub

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

Fix view behavior for AwkwardArrays #1070

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Fix view behavior for AwkwardArrays #1070

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b23eda3
Use __record__ instead of __list__ to store behavior
grst Jul 24, 2023
b640617
Fix views of views
grst Jul 25, 2023
843833f
Update anndata/_core/views.py
grst Jul 25, 2023
8aa09cc
Remove all custom code for views
grst Aug 30, 2023
86c5ff8
WIP add test cases
grst Sep 1, 2023
a01b80e
Merge remote-tracking branch 'origin/main' into ak-array-views
grst Sep 1, 2023
dfe7008
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 1, 2023
8b14e57
More test cases
grst Sep 1, 2023
4870553
Add comment about view behavior
grst Sep 1, 2023
257c063
Merge branch 'main' into ak-array-views
grst Sep 7, 2023
4b34598
Fix akward array view test
grst Sep 8, 2023
fdeff90
Fix that awkward arrays in an awyward array view could not be modified.
grst Sep 8, 2023
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
16 changes: 10 additions & 6 deletions anndata/_core/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from copy import deepcopy
from collections.abc import Sequence, KeysView, Callable, Iterable, Mapping
from functools import reduce, singledispatch, wraps
from typing import Any, Literal
from typing import Any, Literal, Dict, cast
import warnings

import numpy as np
Expand Down Expand Up @@ -295,8 +295,8 @@ def __copy__(self) -> AwkArray:
"""
array = self
# makes a shallow copy and removes the reference to the original AnnData object
array = ak.with_parameter(self, _PARAM_NAME, None)
array = ak.with_parameter(array, "__list__", None)
array = ak.with_parameter(array, _PARAM_NAME, None)
array = ak.with_parameter(array, "__record__", None)
Copy link

Choose a reason for hiding this comment

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

This should use with_name, so that we can traverse into the layout to find the record-node

Suggested change
array = ak.with_parameter(array, "__record__", None)
array = ak.with_name(array, None)

Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine that after our dicsussion @grst could not get that working (see #1040 (comment))

Is there a reason traversing is better than what we have (assuming @grst could not get the with_name working)? We only ever deal with this at a "top level."

return array

@as_view.register(AwkArray)
Expand All @@ -308,15 +308,19 @@ def as_view_awkarray(array, view_args):
# possible strategies to stack behaviors.
# A better solution might be based on xarray-style "attrs", once this is implemented
# https://github.com/scikit-hep/awkward/issues/1391#issuecomment-1412297114
if type(array).__name__ != "Array":
#
# Allow only Arrays that have no record behavior attached, or with __record__ explicitly
# set to None
if array.layout.purelist_parameter("__record__") is not None:
raise NotImplementedError(
"Cannot create a view of an awkward array with __array__ parameter. "
"Cannot create a view of an awkward array with __record__ parameter. "
"Please open an issue in the AnnData repo and describe your use-case."
)
array = ak.with_parameter(array, _PARAM_NAME, (parent_key, attrname, keys))
array = ak.with_parameter(array, "__list__", "AwkwardArrayView")
array = ak.with_parameter(array, "__record__", "AwkwardArrayView")
return array

ak.behavior["*", "AwkwardArrayView"] = AwkwardArrayView
ak.behavior["AwkwardArrayView"] = AwkwardArrayView

except ImportError:
Expand Down
2 changes: 1 addition & 1 deletion anndata/tests/test_awkward.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def reversed(self):
ak.behavior[BEHAVIOUR_ID] = ReversibleArray
adata = gen_adata((3, 3), varm_types=(), obsm_types=(), layers_types=())
adata.obsm["awk_string"] = ak.with_parameter(
ak.Array(["AAA", "BBB", "CCC"]), "__list__", BEHAVIOUR_ID
ak.Array(["AAA", "BBB", "CCC"]), "__record__", BEHAVIOUR_ID
)
adata_view = adata[:2]

Expand Down