-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Avoid calling np.asarray on lazy indexing classes #6874
Changes from all commits
45cd500
9c0350c
74afa53
9de7427
cc0a653
59c7ead
2aa0830
1306758
cf67972
0209900
536648a
f2514c7
3c597d4
201eeba
cd02a8a
7ef55e0
4e77fec
d14c61f
19af950
22db817
2bbcc16
ca2a10a
9256dd0
906c3b3
598c201
941c643
d1127fe
c0c78a1
9b727e6
46d98ec
b19a24b
84f560f
426519f
c4b81bf
d11a3cf
c223617
51552d4
5f1cf53
937d572
cc7d0b5
1576261
7d8459e
9815b75
39e7529
f304bcb
6cb1677
2c7da96
26d224c
65da209
0bc1175
8c2d74c
20c8c81
77f7059
517f195
5c23bd2
887e1c5
2557d02
b313258
cbd030e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -449,13 +449,25 @@ class ExplicitlyIndexed: | |||||||||||||
|
||||||||||||||
__slots__ = () | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype: np.typing.DTypeLike = None) -> np.ndarray: | ||||||||||||||
# Leave casting to an array up to the underlying array type. | ||||||||||||||
return np.asarray(self.get_duck_array(), dtype=dtype) | ||||||||||||||
|
||||||||||||||
def get_duck_array(self): | ||||||||||||||
return self.array | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
class ExplicitlyIndexedNDArrayMixin(NDArrayMixin, ExplicitlyIndexed): | ||||||||||||||
__slots__ = () | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
def get_duck_array(self): | ||||||||||||||
key = BasicIndexer((slice(None),) * self.ndim) | ||||||||||||||
return np.asarray(self[key], dtype=dtype) | ||||||||||||||
return self[key] | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype: np.typing.DTypeLike = None) -> np.ndarray: | ||||||||||||||
# This is necessary because we apply the indexing key in self.get_duck_array() | ||||||||||||||
# Note this is the base class for all lazy indexing classes | ||||||||||||||
return np.asarray(self.get_duck_array(), dtype=dtype) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
class ImplicitToExplicitIndexingAdapter(NDArrayMixin): | ||||||||||||||
|
@@ -467,8 +479,11 @@ def __init__(self, array, indexer_cls=BasicIndexer): | |||||||||||||
self.array = as_indexable(array) | ||||||||||||||
self.indexer_cls = indexer_cls | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
return np.asarray(self.array, dtype=dtype) | ||||||||||||||
def __array__(self, dtype: np.typing.DTypeLike = None) -> np.ndarray: | ||||||||||||||
return np.asarray(self.get_duck_array(), dtype=dtype) | ||||||||||||||
|
||||||||||||||
def get_duck_array(self): | ||||||||||||||
return self.array.get_duck_array() | ||||||||||||||
|
||||||||||||||
def __getitem__(self, key): | ||||||||||||||
key = expanded_indexer(key, self.ndim) | ||||||||||||||
|
@@ -531,9 +546,15 @@ def shape(self) -> tuple[int, ...]: | |||||||||||||
shape.append(k.size) | ||||||||||||||
return tuple(shape) | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
array = as_indexable(self.array) | ||||||||||||||
return np.asarray(array[self.key], dtype=None) | ||||||||||||||
def get_duck_array(self): | ||||||||||||||
array = self.array[self.key] | ||||||||||||||
# self.array[self.key] is now a numpy array when | ||||||||||||||
# self.array is a BackendArray subclass | ||||||||||||||
# and self.key is BasicIndexer((slice(None, None, None),)) | ||||||||||||||
# so we need the explicit check for ExplicitlyIndexed | ||||||||||||||
if isinstance(array, ExplicitlyIndexed): | ||||||||||||||
array = array.get_duck_array() | ||||||||||||||
dcherian marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
return _wrap_numpy_scalars(array) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding xarray/xarray/core/indexing.py Lines 607 to 612 in 3ee7b5a
But now the issue is that we should pass an appropriate Good news is that backends can avoid this complication by returning arrays, so we could just ignore this ugly bit for now. |
||||||||||||||
|
||||||||||||||
def transpose(self, order): | ||||||||||||||
return LazilyVectorizedIndexedArray(self.array, self.key).transpose(order) | ||||||||||||||
|
@@ -584,8 +605,15 @@ def __init__(self, array, key): | |||||||||||||
def shape(self) -> tuple[int, ...]: | ||||||||||||||
return np.broadcast(*self.key.tuple).shape | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
return np.asarray(self.array[self.key], dtype=None) | ||||||||||||||
def get_duck_array(self): | ||||||||||||||
array = self.array[self.key] | ||||||||||||||
# self.array[self.key] is now a numpy array when | ||||||||||||||
# self.array is a BackendArray subclass | ||||||||||||||
# and self.key is BasicIndexer((slice(None, None, None),)) | ||||||||||||||
# so we need the explicit check for ExplicitlyIndexed | ||||||||||||||
if isinstance(array, ExplicitlyIndexed): | ||||||||||||||
array = array.get_duck_array() | ||||||||||||||
return _wrap_numpy_scalars(array) | ||||||||||||||
|
||||||||||||||
def _updated_key(self, new_key): | ||||||||||||||
return _combine_indexers(self.key, self.shape, new_key) | ||||||||||||||
|
@@ -631,8 +659,8 @@ def _ensure_copied(self): | |||||||||||||
self.array = as_indexable(np.array(self.array)) | ||||||||||||||
self._copied = True | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
return np.asarray(self.array, dtype=dtype) | ||||||||||||||
def get_duck_array(self): | ||||||||||||||
return self.array.get_duck_array() | ||||||||||||||
|
||||||||||||||
def __getitem__(self, key): | ||||||||||||||
return type(self)(_wrap_numpy_scalars(self.array[key])) | ||||||||||||||
|
@@ -658,12 +686,14 @@ def __init__(self, array): | |||||||||||||
self.array = _wrap_numpy_scalars(as_indexable(array)) | ||||||||||||||
|
||||||||||||||
def _ensure_cached(self): | ||||||||||||||
if not isinstance(self.array, NumpyIndexingAdapter): | ||||||||||||||
self.array = NumpyIndexingAdapter(np.asarray(self.array)) | ||||||||||||||
self.array = as_indexable(self.array.get_duck_array()) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is the right generalization. |
||||||||||||||
|
||||||||||||||
def __array__(self, dtype: np.typing.DTypeLike = None) -> np.ndarray: | ||||||||||||||
return np.asarray(self.get_duck_array(), dtype=dtype) | ||||||||||||||
|
||||||||||||||
def __array__(self, dtype=None): | ||||||||||||||
def get_duck_array(self): | ||||||||||||||
self._ensure_cached() | ||||||||||||||
return np.asarray(self.array, dtype=dtype) | ||||||||||||||
return self.array.get_duck_array() | ||||||||||||||
|
||||||||||||||
def __getitem__(self, key): | ||||||||||||||
return type(self)(_wrap_numpy_scalars(self.array[key])) | ||||||||||||||
|
@@ -827,7 +857,7 @@ def explicit_indexing_adapter( | |||||||||||||
result = raw_indexing_method(raw_key.tuple) | ||||||||||||||
if numpy_indices.tuple: | ||||||||||||||
# index the loaded np.ndarray | ||||||||||||||
result = NumpyIndexingAdapter(np.asarray(result))[numpy_indices] | ||||||||||||||
result = NumpyIndexingAdapter(result)[numpy_indices] | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not entirely sure about this but it forces us to explicitly pass numpy arrays to NumpyIndexingAdapter. Only the pydap test failed, so I cast those objects to np.array in the backend. |
||||||||||||||
return result | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
|
@@ -1463,6 +1493,9 @@ def __array__(self, dtype: DTypeLike = None) -> np.ndarray: | |||||||||||||
array = array.astype("object") | ||||||||||||||
return np.asarray(array.values, dtype=dtype) | ||||||||||||||
|
||||||||||||||
def get_duck_array(self) -> np.ndarray: | ||||||||||||||
return np.asarray(self) | ||||||||||||||
|
||||||||||||||
@property | ||||||||||||||
def shape(self) -> tuple[int, ...]: | ||||||||||||||
return (len(self.array),) | ||||||||||||||
|
@@ -1603,9 +1636,9 @@ def _repr_inline_(self, max_width: int) -> str: | |||||||||||||
return format_array_flat(self._get_array_subset(), max_width) | ||||||||||||||
|
||||||||||||||
def _repr_html_(self) -> str: | ||||||||||||||
from xarray.core.formatting import short_numpy_repr | ||||||||||||||
from xarray.core.formatting import short_array_repr | ||||||||||||||
|
||||||||||||||
array_repr = short_numpy_repr(self._get_array_subset()) | ||||||||||||||
array_repr = short_array_repr(self._get_array_subset()) | ||||||||||||||
return f"<pre>{escape(array_repr)}</pre>" | ||||||||||||||
|
||||||||||||||
def copy(self, deep: bool = True) -> PandasMultiIndexingAdapter: | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently return
pydap.BaseType
and rely on annp.asarray
call inexplicit_indexing_adapter
. I've removed that call below.