test_rolling_wrapped_dask is failing with dask-master

[shoyer]

The test_rolling_wrapped_dask tests in test_dataarray.py are failing with dask master, e.g., as seen here:
https://travis-ci.org/pydata/xarray/jobs/527936531

I reproduced this locally. git bisect identified the culprit as dask/dask#4756.

The source of this issue on the xarray side appears to be these lines:

xarray/xarray/core/rolling.py

Lines 287 to 291 in dd99b7d

	if isinstance(padded.data, dask_array_type):
	values = dask_rolling_wrapper(func, padded,
	window=self.window,
	min_count=min_count,
	axis=axis)

In particular, we are currently padded as an xarray.DataArray object, not a dask array. Changing this to padded.data shows that passing an actual dask array to dask_array_ops.rolling_window results in failing tests.

@fujiisoup @jhamman any idea what's going on here?

[fujiisoup]

@shoyer

I have no idea yet...
I will look inside later today.

[fujiisoup]

Sent a fix

[shoyer]

Looking into this in more detail, the fix looks somewhat non-trivial. For example, we definitely are not padding by enough inside dask_rolling_wrapper if center=False:

xarray/xarray/core/dask_array_ops.py

Line 29 in 145f25f

depth[axis] = (window + 1) // 2

I'm not sure if this was ever tested properly, at least for dask arrays with multiple chunks. My guess is that this previously worked by consolidating dask arrays into a single chunk, which would simply fail for arrays that are larger than fit into memory.

For now, I think it would be safest to issue a new release that simply disables rolling() methods on dask arrays (by raising an error), and to save a full fix for later. I am concerned about letting the current behavior stick around, which silently calculates the wrong result.

[zbarry]

@shoyer - yeah, I'd tend to agree. I had to do a bit of digging when I encountered this problem, because I was getting serialization errors (since it was doing away with the chunks on massive arrays) that I then eventually connected to this rolling problem. An explicit message that says "can't do this yet" would be good in light of this kind of situation.

[shoyer]

#3040 is my preferred fix for the rolling window issue. I plan to merge it sometime tomorrow unless I get any comments....