issue_comments

@shoyer, is this what you were thinking?

@shoyer, I think this fixes the concerns you raised including the testing. Thanks for all the tips!

@shoyer, this should be ready for another review. I also tested it with this somewhat hacky code at https://gist.github.com/a329d441fe99ae342a34b1a374650138. It may be good to get some type of test like this into the test suite. However, the correct location for testing these methods, in general, is not transparent to me. It doesn't look like we broadly check reduction operations with nans, e.g., prod outside the test_variable.py file. I have made additions here but broader testing may be useful.

@shoyer, @jhamman, and @MaximilianR this should be ready for a preliminary review because it works. The key thing missing is a check for dask version and potentially more testing. Thoughts on these issues are greatly appreciated.

@shoyer and @jhamman here is the general use of the cumsum and cumprod operators. Note, I probably need to have some type of error checking for the dask version (e.g., we require a version of dask with nancumsum, nanprod, and nancumprod). What is the standard way to do this?

For example, dask 0.11.0 works but dask 0.8.1 does not and returns an error.

@jhamman, I need to get back to this when I can make the time but will let you know if I have more trouble. Thanks for following up.

@shoyer, it looks like I'll need to have a _reduce_method-like method that doesn't reduce the dimensions, e.g., https://github.com/pydata/xarray/blob/master/xarray/core/common.py#L11. I haven't been able to get the current branch to work properly (it returns a numpy array vs xarray datatype) and seem to be something missing. Am I on the right track that I need to add a new abstract method? This seems overly complicated but I haven't been able to get this to work cleanly otherwise, e.g., trying things like

python f = _func_slash_method_wrapper(method, name) setattr(cls, name, cls._binary_op(f))

Some advice or help to get me out of my naivete would be greatly appreciated. Thanks!

@shoyer see https://github.com/dask/dask/pull/1077 for the nancumsum and nancumprod dask PR

@mrocklin and @shoyer, will we need to modify the definitions for nanprod, nancumsum, and nancumprod in dask for this to work in multi-threaded mode? I took a quick look and it appears they are defined https://github.com/dask/dask/blob/d82cf2ac3fa3a61912b7934afe7b2fe9e14cc4ff/dask/array/init.py#L17-L22 so I'm assuming xarray/dask should just work once the issues on the xarray end are resolved but just wanted to double check to make sure this is the case.