issue_comments

Thanks @Illviljan, @dcherian, and @keewis so much for the help.

@dcherian Do you think it would be better to finish this PR as the creation of _aggregations.py to give the cum methods better documentation? Then start a new one to fix #6528?

I've merged the cumulative and reduction files into generate_aggregations.py and _aggregations.py. This uses the original version of reductions with an additional statement on the dataset methods that adds the original coordinates back in.

Using apply_ufunc and np.cumsum/cumprod has some issues as it only finds the cumulative across one axis which makes iterating through each dimension necessary. This makes it slower than the original functions and also causes some problems with the groupby method.

Happy for any input on how the method using apply_ufunc might be usable or on any ways to change the current method.

I'm getting a few issues I don't quite understand: - When running pytest on my local repository I get no errors but it's failing the checks here with a NotImplementedError - Black is having an issue with some of the strings in generate_aggregations. It's saying it cannot parse what should be valid code.

Thanks!

python def cumsum(..., dim): return xr.apply_ufunc( np.cumsum if skipna else np.nancumsum, obj, input_core_dims=[dim], output_core_dims=[dim], kwargs={"axis": -1}, ) # now transpose dimensions back to input order

I'm running into an issue with variables without the core dimensions. Would it be better to do a work around in cumsum or in apply_unfunc like you mentioned in #6391

Thanks @dcherian, I'll try to work that in. Is there a particular reason why there is no cumprod for GroupBy objects?

Great I'll start working on that. Shouldn't take too long

@Illviljan That is definitely something I could do. Are there any other methods I should be including in this?

Thanks @max-sixty

Here's a fix of the type checker and a small suggestion (feel free to accept or reject!)

```diff commit 5dad26e19f1fb22e9a31854bac9471c5e1bec43c Author: Maximilian Roos m@maxroos.com Date: Tue Oct 4 00:26:48 2022 -0700

Ignore type on intentional error; (optional) tweak to error message

diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index ff54bd84..a3f26d26 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -5405,7 +5405,7 @@ def transpose( if (len(dims) > 0) and (isinstance(dims[0], list)): list_fix = [f"'{x}'" if isinstance(x, str) else f"{x}" for x in dims[0]] raise TypeError( - f'transpose requires dims to be passed as multiple arguments. Expected "{", ".join(list_fix)}". Received "{dims[0]}" instead' + f'transpose requires dims to be passed as multiple arguments. Expected .transpose({", ".join(list_fix)}). Received .transpose({dims[0]}) instead' ) This is what I initially had but I thought it might be confusing for users passing something to missing_dimsdiff

-def test_traspose_error() -> None: +def test_transpose_error() -> None: # Transpose dataset with list as argument # Should raise error ds = xr.Dataset({"foo": (("x", "y"), [[21]]), "bar": (("x", "y"), [[12]])}) @@ -6817,7 +6817,7 @@ def test_traspose_error() -> None: with pytest.raises( TypeError, match=re.escape( - "transpose requires dims to be passed as multiple arguments. Expected \"'y', 'x'\". Received \"['y', 'x']\" instead" + "transpose requires dims to be passed as multiple arguments. Expected .transpose('y', 'x'). Received .transpose(['y', 'x']) instead" ), ): - ds.transpose(["y", "x"]) + ds.transpose(["y", "x"]) # type: ignore ``` This is very helpful thank you!