issue_comments

Thanks for all the help!

OK, in it goes. Thanks @max-sixty !

Updated! Let me know any final changes!

Great - updated! Let me know any final comments!

The gentlest of reminders that I think this is ready to merge (mea culpa for leaving it at 90% for so long)

This is updated! Could put an "Experimental" label on if we want (or maybe that's implicit).

Let me know any final changes. Will be good to get this merged at last.

Can you add some docs to http://xarray.pydata.org/en/stable/computation.html#rolling-window-operations?

Any thoughts on the API?

@shoyer is making some updates to numbagg so as soon as those are complete, we could point the dependencies at that release and merge this

I made an attempt to add the "skip variables without the dimension" to apply_ufunc, but it's much harder than I expected - there are more cases than I expected (e.g. multiple datasets).

I may be missing something - let me know if there's an reasonable approach

Otherwise I'll do the close thing for this PR, and potentially we can have a look at the general solution later

@max-sixty

The error is when applying over a dimension on a dataset where only some of the variables have the dimension;

I remember I faced the same issue in implementing differentiate, interp, trapz, etc... and manually wrote the same logic for several times.

If it's helpful to add that functionality directly to apply_ufunc, lmk.

I think it would make the code much cleaner at least for these methods.

That said, if EWM preserves the dimension size/labels you probably don't nee any special logic for coordinates

The error is when applying over a dimension on a dataset where only some of the variables have the dimension; e.g. applying over time on this:

python <xarray.Dataset> Dimensions: (time: 10, x: 8, y: 2) Coordinates: * x (x) float64 0.0 0.1429 0.2857 0.4286 0.5714 0.7143 0.8571 1.0 * time (time) float64 0.0 0.1111 0.2222 0.3333 ... 0.7778 0.8889 1.0 c (y) <U1 'a' 'b' * y (y) int64 0 1 Data variables: z1 (y, x) float64 -0.1035 -0.8153 -1.583 ... 1.447 0.7768 -0.2699 z2 (time, y) float64 0.968 0.7156 -1.64 ... 0.1889 -1.142 0.7172

...rather than any issues applying on coords. I think the solution is easy - only apply on variables where the dimension exists.

If it's helpful to add that functionality directly to apply_ufunc, lmk. I'll focus on getting this working, though

Does anyone have a view on the canonical way to implement these for Dataset, given potentially only a subset of the variables will have the dimension? Tests fail when naively using apply_ufunc; .reduce looks like it has some functionality for skipping those variables. Or I could do it manually in a couple of lines.

You could probably copy the logic from Dataset.reduce, which simply applies different logic for coordinates (they either get preserved or dropped depending on if they reuse the reduced dimension). That said, if EWM preserves the dimension size/labels you probably don't nee any special logic for coordinates -- see DatasetRolling.reduce as well.

I think this is in a reasonable state for DataArray, excluding docs. Let me know any feedback on the APi

Does anyone have a view on the canonical way to implement these for Dataset, given potentially only a subset of the variables will have the dimension? Tests fail when naively using apply_ufunc; .reduce looks like it has some functionality for skipping those variables. Or I could do it manually in a couple of lines.

see https://github.com/pydata/xarray/pull/2669 for the tests issue

On Sun, Jan 13, 2019 at 6:12 PM Maximilian Roos notifications@github.com wrote:

Tests seem to be failing on a different issue? https://travis-ci.org/pydata/xarray/jobs/479042667#L7759

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/2650#issuecomment-453842477, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1qKW-0elts0N53ojvQWe1oO3PoRIks5vC1rrgaJpZM4Zt5Sw .

Tests seem to be failing on a different issue? https://travis-ci.org/pydata/xarray/jobs/479042667#L7759

BTW, does ewm computes the window mean based on index or coordinate value?

Index, currently.

Would be great to have an algo that dealt with coord value, and I think not too difficult

I would lean towards a dedicated method, since there are method specific options. It's pretty awkward to reuse a single interface for that.

I like window_type / weighting argument to Rolling. But a little concern is whether it matches to the current API, such as __iter__.

BTW, does ewm computes the window mean based on index or coordinate value?

I know the name ewm matches pandas, but I find it rather inscrutable if you don't already know the acronym.

👍

What about something a little longer, maybe exp_rolling()?

Yes, that works. One alternative is to add a window_type / weighting argument to Rolling - Exponentially weighted is one of many alternatives

I'm fairly balanced between them - others' thoughts?

Before we add even an optional dependency on numbagg in xarray, we should probably do a bit of cleanup (e.g., making sure we're happy with its public interface, and putting a release up on pypi)

I know the name ewm matches pandas, but I find it rather inscrutable if you don't already know the acronym. What about something a little longer, maybe exp_rolling()?