issue_comments

Okay, _full_like (formerly empty_like) has been made private. I think it will be a useful feature for a bunch of applications but its not the point here. I'd like to build out functionality akin to what @shoyer indicated (zeros_like, ones_like, and missing_like). Actually, that can all be done by _full_like as it stands but it sounds like we need to put some more thought into the API before making it public.

An example of how we differ from Pandas in the last position with center=True. This stems from how we "center" the data using shift.

``` Python In [7]: arr Out[7]: <xarray.DataArray (y: 5)> array([ 2.5, 3. , 3.5, 4. , 4.5]) Coordinates: x int64 1 * y (y) int64 0 1 2 3 4

In [8]: arr.rolling(y=3, center=True, min_periods=1).mean() Out[8]: <xarray.DataArray (y: 5)> array([ 2.75, 3. , 3.5 , 4. , nan]) Coordinates: x int64 1 * y (y) int64 0 1 2 3 4

In [9]: pd.rolling_mean(arr.to_series(), 3, center=True, min_periods=1) Out[9]: y 0 2.75 1 3.00 2 3.50 3 4.00 4 4.25 dtype: float64 ```

@shoyer -

This could use another review from you. Failing tests have been fixed. There's a bunch more functionality that can be built out in future pull requests. This provides the basic rolling functionality we were going for.

One thing to note, with center=True, we differ from pandas in the last position in some instances. I made a note in my unit test that we're okay with that for now.

~~Lastly, this will need a squash and I'll do that once we're settled on the features.~~

I'll give this a test, but it looks like you have all the pieces to me....

I'm getting this TypeError:

Python E TypeError: move_sum() takes at least 2 positional arguments (0 given)

which makes me think the injection is failing to pass the arguments in

For iteration, what about only iterating over full windows? Thinking about how I might use iteration, I think this might be more useful than returning some shrunk windows.

I did consider this at first and it wouldn't be all that hard to implement but I chose not to go this route because I wanted consistency between reduce, _bottleneck_reduce and __iter__. In theory, all three of these should provide the same answer:

``` Python rolling_obj = da.rolling(time=4)

rolling_obj.mean() # bottleneck move_mean rolling_obj.reduce(np.nanmean) # numpy nanmean over each window concat([da.mean(dim='time') for _, da in rolling_obj], dim=rolling_obj.window_labels) # manual mean via iterable - same as reduce ```

I think you've done a pretty reasonable job of interpreting min_periods for iteration, but I would still vote for defining it only as an argument to the aggregation methods and not worrying about it for iteration.

How did pandas land on this. To me it makes more sense as an argument to __init__ but I'll go with whatever pandas decided for consistency.

sounds good. Thanks. That's got to slow down work at a tech company :zzz:

@shoyer - would you mind taking a look at what I've just tried (and failed) in ops.py and common.py? I think I'm missing a big piece of the injection puzzle.

@shoyer - I made some more progress here tonight. How do you suggest we handle the bottleneck dependency? That is the reason for the failing tests at the moment.

@shoyer - thanks for the first look. I'll give it another hack.