Clean-up indexing adapter classes #10355
Conversation
Grouping the logic into one method will make it easier overriding the behavior in subclasses (interval index) without affecting much readability. Also it yield more DRY code.
Prevent numpy.dtype conversions or castings implemented in various places, gather the logic into one method.
Maybe slice PandasIndexingAdapter / CoordinateTransformAdapter before formatting them as arrays. For PandasIndexingAdapter, this prevents converting a large pd.RangeIndex into a explicit index or array.
|
|
||
| result = self.array[key] | ||
| def _get_array_subset(self) -> np.ndarray: |
There was a problem hiding this comment.
So this is only to share code between PandasIndexingAdapter and PandasMultiIndexAdapter?
There was a problem hiding this comment.
This is now used directly in PandasIndexingAdapter to avoid converting a large pd.RangeIndex into a numpy array just for showing the first and last few values.
In #10296 it is also used (overridden in PandasIntervalIndexingAdapter) to avoid converting a large pd.IntervalIndex into a 2-d array. EDIT: actually it isn't (yet).
| @@ -651,6 +655,12 @@ def short_array_repr(array): | |||
| def short_data_repr(array): | |||
| """Format "data" for DataArray and Variable.""" | |||
| internal_data = getattr(array, "variable", array)._data | |||
|
|
|||
| if isinstance( | |||
| internal_data, PandasIndexingAdapter | CoordinateTransformIndexingAdapter | |||
There was a problem hiding this comment.
Can't we get away with adding_repr_inline to CoordinateTransfromIndexingAdapter? I think it's preferable to avoid this kind of special casing.
There was a problem hiding this comment.
This function does not format the inline repr but the data of a DataArray or variable.
We could add a short_data_repr method to those adapter classes and check if internal_data has such a method here, although it is not much different from this special casing.
There was a problem hiding this comment.
I guess a better request is to see whether we can just reuse format_array_flat which already does indexing and should just work for these classes.
There was a problem hiding this comment.
Hmm again format_array_flat is for formatting the inline repr, whereas the special case introduced here is for formatting the data repr.
Without this special case, short_data_repr() will convert the indexing adapters into numpy arrays via PandasIndexingAdpater.get_duck_array() and CoordinateTransformIndexingAdapter.get_duck_array() over their full shape / size. For both the inline and the data reprs, we want to select only the first and last relevant items before doing this conversion.
first_n_items() and last_n_items() in xarray.core.formatting do similar things than PandasIndexingAdapter._get_array_subset() and CoordinateTransformIndexingAdapter._get_array_subset(). We could perhaps reuse the two former instead, although for the data repr (at least for CoordinateTransform, and possibly later for pd.IntervalIndex) we don't want a flattened result. So this would involve more refactoring. Also this wouldn't remove the special case here.
Alternatively we could tweak Variable._in_memory such that it returns False when ._data is a PandasIndexingAdapter (only when it wraps a pd.RangeIndex) or a CoordinateTransformIndexingAdapter, which will turn their data repr from, e.g.,
>>> xidx = PandasIndex(pd.RangeIndex(2_000_000_000), "x")
>>> ds = xr.Dataset(coords=xr.Coordinates.from_xindex(xidx))
>>> print(ds.x.variable)
<xarray.IndexVariable 'x' (x: 2000000000)> Size: 16GB
array([[ 0, 1, 2, ..., 1999999997, 1999999998,
1999999999]])into
>>> print(ds.x.variable)
<xarray.IndexVariable 'x' (x: 2000000000)> Size: 16GB
[2000000000 values with dtype=dtype('int64')]On one hand I like seeing the actual first and last values of a (lazy) range index or coordinate transform. On the other hand I find a bit confusing that is it shown like a plain numpy array.
whats-new.rstMostly cherry-picked from #10296 (cleaner in a separate PR).
This PR also prevents converting a whole
pd.RangeIndexor aCoordinateTransformobject into a numpy array when showing the array repr. The object is sliced / indexed into a smaller one before doing the conversion. For large indexes this can make a big difference:However, I'm now wondering how we should format the array (inline) repr for those two cases. It is useful to show some concrete values but at the same time it is misleading to show them like plain numpy arrays.
It would certainly help to have some visual mark for lazy variables (perhaps next to their size), but that's another topic.