Fix handling empty slices of a tensor - #802
Conversation
…o remove code duplication; test added
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #802 +/- ##
===========================================
+ Coverage 85.96% 85.98% +0.01%
===========================================
Files 173 174 +1
Lines 13450 13467 +17
Branches 674 676 +2
===========================================
+ Hits 11562 11579 +17
Misses 1698 1698
Partials 190 190 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| def _to_array(self, method, index=None, **kwargs): | ||
| arr, current_field_shape = self._prepare_tensor_data(method, index=index, **kwargs) | ||
| if arr is None: |
There was a problem hiding this comment.
This should be array format agnostic. The array_namespace is ideally in the kwargs.
| import earthkit.utils.array.namespace.numpy as xp | ||
|
|
||
| arr = xp.empty(shape=0, dtype="f8") | ||
| return arr.reshape(self.user_shape + current_field_shape) |
There was a problem hiding this comment.
The reshape on the array namespace should be used.
There was a problem hiding this comment.
Do you mean using the top-level function xp.reshape(arr, new_shape) for an array namespace xp rather than arr.reshape(new_shape) ?
There was a problem hiding this comment.
Yes, exactly. We try to use all these methods on the array namespace (when it supports the methods). Both empty and reshape are part of the array api.
…hrough the method's identifier; xr slice test renamed
…case fixed; FieldList.to_latlon(): index kwarg taken into account
…ay instead of `None`
Description
This PR fixes a problem in handling empty slices of Xarray objects whenever the underlying data in managed by a field list through a
FieldListTensorobject (e.g. when a GRIB file is converted to Xarray via.to_xarray(lazy_load=True, ...)method). Consider the following code:The problem with
ds2.load()comes from the fact that the underlying field list is empty (i.e.None, seeearthkit-data/src/earthkit/data/core/fieldlist.py
Line 1108 in 195942d
This PR fixes this issue by properly handling the case of an empty field list, see:
earthkit-data/src/earthkit/data/indexing/tensor.py
Line 433 in dbc9810
A related code was refactored slightly to remove some code duplication between
to_numpy,to_arraymethods withinFieldListTensorandFieldListSparseTensorclasses.A relevant test was added.
Contributor Declaration
By opening this pull request, I affirm the following: