rolling_exp (nee ewm)

[max-sixty]

• Duplicate groupby / rolling interface
• Integrate with apply_ufunc, including orientation and coords
• Define com / span / alpha interface
• Implement for Variable & Dataset
• Docs

[shoyer]

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()?

[shoyer]

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)

[max-sixty]

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?

[fujiisoup]

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?

[shoyer]

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

[max-sixty]

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

[max-sixty]

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

[shoyer]

see #2669 for the tests issue

…

On Sun, Jan 13, 2019 at 6:12 PM Maximilian Roos ***@***.***> 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 <#2650 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABKS1qKW-0elts0N53ojvQWe1oO3PoRIks5vC1rrgaJpZM4Zt5Sw> .

[max-sixty]

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.

[shoyer]

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.

[max-sixty]

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:

<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

[fujiisoup]

@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.

[max-sixty]

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]

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.

[max-sixty]

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

[pep8speaks]

Hello @max-sixty! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-06-24 03:44:56 UTC

[max-sixty]

Great - updated! Let me know any final comments!

[max-sixty]

Updated! Let me know any final changes!

[shoyer]

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

[max-sixty]

Thanks for all the help!