Use duck array ops in more places #8267

tomwhite · 2023-10-03T13:28:25Z

This uses duck_array_ops in some more places that were found to cause failures in cubed-dev/cubed-xarray#8

Closes #xxxx
Tests added
User visible changes (including notable bug fixes) are documented in whats-new.rst
New functions/methods are listed in api.rst

for more information, see https://pre-commit.ci

TomNicholas

Thanks for these additions @tomwhite ! Obviously I missed these ones.

Don't forget to add a note in whatsnew!

TomNicholas · 2023-10-03T15:05:45Z

xarray/tests/test_coarsen.py

@@ -6,6 +6,7 @@

 import xarray as xr
 from xarray import DataArray, Dataset, set_options
+from xarray.core import duck_array_ops


This effectively changes the test from only testing public xarray API to now relying on some internals too, which is not ideal from a code separation perspective.

In practice I think this is probably fine here, and I know you're doing it to allow Cubed arrays to go through the modified tests, but just something to bear in mind.

Illviljan

Isn't it better to get used to using reshape directly?

Illviljan · 2023-10-04T19:52:32Z

xarray/core/accessor_dt.py

@@ -50,7 +51,7 @@ def _access_through_cftimeindex(values, name):
    from xarray.coding.cftimeindex import CFTimeIndex

    if not isinstance(values, CFTimeIndex):
-        values_as_cftimeindex = CFTimeIndex(values.ravel())
+        values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))


Suggested change

values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))

values_as_cftimeindex = CFTimeIndex(duck_array_ops.reshape(values, (-1,)))

Can't we just get used to using reshape instead?

Illviljan · 2023-10-04T19:52:50Z

xarray/core/accessor_dt.py

@@ -69,7 +70,7 @@ def _access_through_series(values, name):
    """Coerce an array of datetime-like values to a pandas Series and
    access requested datetime component
    """
-    values_as_series = pd.Series(values.ravel(), copy=False)
+    values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)


Suggested change

values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)

values_as_series = pd.Series(duck_array_ops.reshape(values, (-1,)), copy=False)

Illviljan · 2023-10-04T19:53:29Z

xarray/core/accessor_dt.py

@@ -148,10 +149,10 @@ def _round_through_series_or_index(values, name, freq):
    from xarray.coding.cftimeindex import CFTimeIndex

    if is_np_datetime_like(values.dtype):
-        values_as_series = pd.Series(values.ravel(), copy=False)
+        values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)


Suggested change

values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)

values_as_series = pd.Series(duck_array_ops.reshape(values, (-1,)), copy=False)

Illviljan · 2023-10-04T19:53:50Z

xarray/core/accessor_dt.py

        method = getattr(values_as_series.dt, name)
    else:
-        values_as_cftimeindex = CFTimeIndex(values.ravel())
+        values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))


Suggested change

values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))

values_as_cftimeindex = CFTimeIndex(duck_array_ops.reshape(values, (-1,)))

Illviljan · 2023-10-04T19:54:08Z

xarray/core/accessor_dt.py

@@ -195,7 +196,7 @@ def _strftime_through_cftimeindex(values, date_format: str):
    """
    from xarray.coding.cftimeindex import CFTimeIndex

-    values_as_cftimeindex = CFTimeIndex(values.ravel())
+    values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))


Suggested change

values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))

values_as_cftimeindex = CFTimeIndex(duck_array_ops.reshape(values, (-1,)))

Illviljan · 2023-10-04T19:54:23Z

xarray/core/accessor_dt.py

@@ -205,7 +206,7 @@ def _strftime_through_series(values, date_format: str):
    """Coerce an array of datetime-like values to a pandas Series and
    apply string formatting
    """
-    values_as_series = pd.Series(values.ravel(), copy=False)
+    values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)


Suggested change

values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)

values_as_series = pd.Series(duck_array_ops.reshape(values, (-1,)), copy=False)

Illviljan · 2023-10-04T19:54:40Z

xarray/core/computation.py

@@ -2123,7 +2123,8 @@ def _calc_idxminmax(
        chunkmanager = get_chunked_array_type(array.data)
        chunks = dict(zip(array.dims, array.chunks))
        dask_coord = chunkmanager.from_array(array[dim].data, chunks=chunks[dim])
-        res = indx.copy(data=dask_coord[indx.data.ravel()].reshape(indx.shape))
+        data = dask_coord[duck_array_ops.ravel(indx.data)]


Suggested change

data = dask_coord[duck_array_ops.ravel(indx.data)]

data = dask_coord[duck_array_ops.reshape(indx.data, (-1,))]

Illviljan · 2023-10-04T19:55:00Z

xarray/core/duck_array_ops.py

+def ravel(array):
+    return reshape(array, (-1,))
+
+


Suggested change

def ravel(array):

return reshape(array, (-1,))

dcherian · 2023-10-05T18:41:14Z

Thanks @Illviljan I think using ravel is a bit more readable and newcomer friendly.

* upstream/main: (46 commits) xfail flaky test (pydata#8299) Most of mypy 1.6.0 passing (pydata#8296) Rename `reset_encoding` to `drop_encoding` (pydata#8287) Enable `.rolling_exp` to work on dask arrays (pydata#8284) Fix `GroupBy` import (pydata#8286) Ask bug reporters to confirm they're using a recent version of xarray (pydata#8283) Add pyright type checker (pydata#8279) Improved typing of align & broadcast (pydata#8234) Update ci-additional.yaml (pydata#8280) Fix time encoding regression (pydata#8272) Allow a function in `.sortby` method (pydata#8273) make more args kw only (except 'dim') (pydata#6403) Use duck array ops in more places (pydata#8267) Don't raise rename warning if it is a no operation (pydata#8266) Mandate kwargs on `to_zarr` (pydata#8257) copy the `dtypes` module to the `namedarray` package. (pydata#8250) Add xarray-regrid to ecosystem.rst (pydata#8270) Use strict type hinting for namedarray (pydata#8241) Update type annotation for center argument of dataaray_plot methods (pydata#8261) [pre-commit.ci] pre-commit autoupdate (pydata#8262) ...

tomwhite added 4 commits October 3, 2023 14:23

Use duck array ops for reshape

2fba050

Use duck array ops for sum

46c5ace

Use duck array ops for astype

a9509f2

Use duck array ops for ravel

c83f3d8

github-actions bot added the topic-arrays related to flexible array support label Oct 3, 2023

[pre-commit.ci] auto fixes from pre-commit.com hooks

92b5430

for more information, see https://pre-commit.ci

TomNicholas approved these changes Oct 3, 2023

View reviewed changes

Update what's new

6f3bf0d

dcherian approved these changes Oct 3, 2023

View reviewed changes

Illviljan reviewed Oct 4, 2023

View reviewed changes

dcherian merged commit bd40c20 into pydata:main Oct 5, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use duck array ops in more places #8267

Use duck array ops in more places #8267

tomwhite commented Oct 3, 2023 •

edited

Loading

TomNicholas left a comment

TomNicholas Oct 3, 2023

Illviljan left a comment

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

Illviljan Oct 4, 2023

dcherian commented Oct 5, 2023

	values_as_cftimeindex = CFTimeIndex(duck_array_ops.ravel(values))
	values_as_cftimeindex = CFTimeIndex(duck_array_ops.reshape(values, (-1,)))

	values_as_series = pd.Series(duck_array_ops.ravel(values), copy=False)
	values_as_series = pd.Series(duck_array_ops.reshape(values, (-1,)), copy=False)

	data = dask_coord[duck_array_ops.ravel(indx.data)]
	data = dask_coord[duck_array_ops.reshape(indx.data, (-1,))]

Use duck array ops in more places #8267

Use duck array ops in more places #8267

Conversation

tomwhite commented Oct 3, 2023 • edited Loading

TomNicholas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Illviljan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcherian commented Oct 5, 2023

tomwhite commented Oct 3, 2023 •

edited

Loading