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


 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


    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

    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


    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

    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

    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

        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

	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,))]

Uh oh!

Use duck array ops in more places #8267

Use duck array ops in more places #8267

Uh oh!

Conversation

tomwhite commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomNicholas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Illviljan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcherian commented Oct 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tomwhite commented Oct 3, 2023 •

edited

Loading