fix #3141 add cumsum to DatasetGroupBy

[VladSkripniuk]

• Closes calculating cumsums on a groupby object #3141
• Tests added
• Passes black . && mypy . && flake8

[max-sixty]

Any idea why group_id becomes a data variable rather than a coord?

Here's the equivalent with sum:

In [6]: ds.groupby("group_id").sum()
Out[6]:
<xarray.Dataset>
Dimensions:   (group_id: 3)
Coordinates:
  * group_id  (group_id) int64 0 1 2
Data variables:
    foo       (group_id) int64 10 2 3

[VladSkripniuk]

group_id is removed from coords in this line

xarray/xarray/core/merge.py

Line 363 in 6b70107

coords.discard(name)

If we comment this line and run our example again, group_id appears in coords after cumsum. I need some help to understand what this line does.

[max-sixty]

I could look more.

Does anyone who have more experience with this code know what's happening?

[pgierz]

Any progress on this one? I would like to use this functionality. I have a workaround for right now, but it would be nice to do it "correctly"

[max-sixty]

I'm very sorry this didn't get attention — it was a good PR and it's bad of us to drop something from a first-time contributor.

It was surprising that it promoted the group_id variable to a data_var from a coord — I think that's probably a mistake. So if we could resolve that — or show that it would have other adverse impacts — we could merge this.

The good news is that we simplified how methods are supplied to the GroupBy objects — the bad news is that it requires a small rewrite of the code now; though not the tests, which are the majority of this code.

[Illviljan]

Aren't we soon going to let flox handle this once #5734 is merged? numpy_groupies supports cumsum at least so it might be possible to support it in flox as well.

[max-sixty]

Aren't we soon going to let flox handle this once #5734 is merged? numpy_groupies supports cumsum at least so it might be possible to support it in flox as well.

Ah, great, if flox supports cumsum then that is indeed ideal.

[dcherian]

It doesn't at the moment. Writing the dask version will be fun :).

We can always add groupby().cumsum and use the "old" codepath like we do for median.

[dcherian]

I opened #6525 preserving commit authorship. Does that look right to everyone? I could force-push here but was afraid of losing the author info.

That PR doesn't actually work, it's not clear to me why.

EDIT: OK it works now, but we have the same error about losing an index

[max-sixty]

Thanks a lot @dcherian !