issue_comments

@shyer, do you have further suggestions? I think this is almost ready.

@shoyer , thanks for the detailed review.

I noticed the benchmark test is still failing. After fixing this, I will merge this.

Thanks :)

@shoyer , thanks for the review.

These names are very descriptive, but unfortunately they are different for DataArray and Dataset, which means that users will need to write different code for different xarray types.

Agreed. Changed to_dataarray / to_dataset -> construct.

What's the current status here vs #1883? Do you still want to merge that one first?

Yes. It looks I reverted to a wrong commit. I will overwrite most modifications in duck_array_ops and test_duck_array_ops after #1883 is merged..

I sent a separate PR (#1883) for nan-reduce methods. I will continue / finish up this PR after #1883 is merged.

Hmm... The support of nansum for object-dtype is much more difficult than I expected.

Now, I am stacked in supporting numpy<1.13. With numpy<1.13, nansum for the object type gives just a nan.

nansum scripts defined in numpy==1.13 does not help because != operator used here behaves differently in numpy<1.13.

I think our best choice is to revert this PR to 4189d71 and left this issue to #1866.

Although nan-reduce methods (such as nanmean) might be sped up by replacing nan by a certain number before constructing a rolling window, this improvement brings another complexity into the code (and bottleneck already does this).

I want to finish this PR first and left such improvements to another chance. Please let me know if anyone has comments of this PR.

I think this is ready for the final review, including docs. I would like to add rolling methods to airspeed benchmark, but in another PR.

During the work, I notice a somehow unexpected behavior of rolling.

I expected that with min_periods=1 option, we will get an array without nan, It is true with center=False and also pd.rolling.

```python In [2]: da = xr.DataArray(np.arange(10), dims='x') In [3]: da.rolling(x=3, min_periods=1, center=False).sum() Out[3]: <xarray.DataArray (x: 10)> array([ 0., 1., 3., 6., 9., 12., 15., 18., 21., 24.]) Dimensions without coordinates: x

In [5]: s = pd.Series(np.arange(10)) s.rolling(3, min_periods=1, center=True).sum() Out[7]: 0 1.0 1 3.0 2 6.0 3 9.0 4 12.0 5 15.0 6 18.0 7 21.0 8 24.0 9 17.0 dtype: float64 But with `center=True`, we have a `nan` at the end. In [4]: da.rolling(x=3, min_periods=1, center=True).sum() Out[4]: <xarray.DataArray (x: 10)> array([ 1., 3., 6., 9., 12., 15., 18., 21., 24., nan]) Dimensions without coordinates: x ```

It is because we make shift operation after the rolling. https://github.com/pydata/xarray/blob/74d8318c68be884134d449afad18dfe731d48b72/xarray/core/rolling.py#L263-L269

If we pad the array before the rolling operation instead of shift, we will not get the last nan and the result would be the same to pandas. (rolling.to_dataarray('window_dim') does this).

I think this path is more intuitive. Any thoughts?