Change .groupby fastpath to work for monotonic increasing and decreasing

[JoelJaeschke]

This fixes GH6220 which makes it possible to use the fastpath for .groupby for monotonically increasing and decreasing values.

• Closes [FEATURE]: Use fast path when grouping by unique monotonic decreasing variable #6220
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

[dcherian]

Thanks @JoelJaeschke!

The intent was to change the code in _unique_and_monotonic here

A good test would be make sure you get the same answer when grouping a DataArray of ones by a monotonically increasing variable (e.g. [1, 1, 2, 2, 3, 3, 3]) and the decreasing variable( e.g. [3, 3, 3, 2, 2, 1, 1])

[JoelJaeschke]

Hi @dcherian
that was so obvious that I missed it..My bad!

I am not sure if I understand correctly what you are saying, but if I implement the test as you describe, would this not already work anyways? It will take the slow-path, but the behavior should be identical, right? My intention when writing the test was to explicitly test this case by supplying a grouper.

[dcherian]

TO check the fastpath we'd want to both get the right answer and make sure that _group_indices is filled with slices:

xarray/xarray/core/groupby.py

Lines 422 to 430 in 4f3128b

	elif group.dims == (group.name,) and _unique_and_monotonic(group):
	# no need to factorize
	if not squeeze:
	# use slices to do views instead of fancy indexing
	# equivalent to: group_indices = group_indices.reshape(-1, 1)
	group_indices = [slice(i, i + 1) for i in range(group.size)]
	else:
	group_indices = np.arange(group.size)
	unique_coord = group

[JoelJaeschke]

Hi @dcherian, I implemented the feedback as you described, hope this does the trick.

[dcherian]

Thanks @JoelJaeschke Can you add a line to doc/whats-new.rst please?

[dcherian]

I undid this change. It's more involved to make it actually work.

[dcherian]

nice test!

I just added a value check on the result below.

[dcherian]

new test to make sure we error when trying to resample with a decreasing time index.

[JoelJaeschke]

Hi @dcherian, I added a note to whats-new. Thanks for your patience, still got a lot to learn :D

[Illviljan]

This one is failing on CI / ubuntu-latest py3.9 bare-minimum, is it taking a path without flox installed perhaps?

___________ TestDataArrayGroupBy.test_groupby_fastpath_for_monotonic ___________
[gw2] linux -- Python 3.9.16 /home/runner/micromamba-root/envs/xarray-tests/bin/python

self = <xarray.tests.test_groupby.TestDataArrayGroupBy object at 0x7fbd9f461c10>

    def test_groupby_fastpath_for_monotonic(self):
        # Fixes https://github.com/pydata/xarray/issues/6220
        index = [1, 2, 3, 4, 7, 9, 10]
        array = DataArray(np.arange(len(index)), [("idx", index)])
        array_rev = array.copy().assign_coords({"idx": index[::-1]})
        fwd = array.groupby("idx", squeeze=False)
        rev = array_rev.groupby("idx", squeeze=False)
    
        for gb in [fwd, rev]:
            assert all([isinstance(elem, slice) for elem in gb._group_indices])
    
        assert_identical(fwd.sum(), array)
>       assert_identical(rev.sum(), array_rev.sortby("idx"))
E       AssertionError: Left and right DataArray objects are not identical
E       
E       Differing values:
E       L
E           array([0, 1, 2, 3, 4, 5, 6])
E       R
E           array([6, 5, 4, 3, 2, 1, 0])
E       Differing coordinates:
E       L * idx      (idx) int64 10 9 7 4 3 2 1
E       R * idx      (idx) int64 1 2 3 4 7 9 10

[welcome]

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again!

[dcherian]

Thanks @JoelJaeschke sorry for the really long delay here!