pydata / xarray

N-D labeled arrays and datasets in Python
https://xarray.dev
Apache License 2.0
3.63k stars 1.09k forks source link

`rolling.construct`: Add `sliding_window_view_kwargs` to pipe arguments down to `sliding_window_view` #9720

Closed dcherian closed 3 days ago

dcherian commented 2 weeks ago

Closes #9550 xref #4325

cc @phofl

~Blocked till the next dask release, so we can verify tests~

max-sixty commented 2 weeks ago

I don't have much context here, but should we always do this if possible, rather than add an option? Or we want to allow an escape hatch for instances where copies are made?

dcherian commented 2 weeks ago

It is on by default in the public APIs (construct and reduce). Advanced users can opt-out. For internal uses, where copies are not made, I set it to False by default (in _reduce_method) and opt-in to rechunking when necessary for each individual method.

I think we should expose it in construct since that is a thin wrapper around sliding_window_view. Are you concerned about exposing it in reduce?

max-sixty commented 2 weeks ago

No! Generally keen on keeping the API small but totally reasonable here!

dcherian commented 2 weeks ago

@max-sixty one option would be to add sliding_window_view_kwargs to forward arbitrary kwargs down to the implementing library. For example, numpy has writeable which someone might want to set (we do not allow this currently)

dcherian commented 1 week ago

I went with sliding_window_kwargs to save some typing...

dcherian commented 6 days ago

@max-sixty any more thoughts here?