Repository: pydata/xarray

Issue:
[FEATURE]: Use fast path when grouping by unique monotonic decreasing variable
### Is your feature request related to a problem?

See https://github.com/pydata/xarray/pull/6213/files#r795716713

We check whether the `by` variable for groupby is unique and monotonically increasing. But the fast path would also apply to unique and monotonically decreasing variables.

### Describe the solution you'd like

Update the condition to `is_monotonic_increasing or is_monotonic_decreasing` and add a test.

### Describe alternatives you've considered

_No response_

### Additional context

_No response_

Discussion:
dcherian: Thanks @JoelJaeschke The intent was to change the code in `_unique_and_monotonic`.

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? 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:
https://github.com/pydata/xarray/blob/4f3128bb7266d1a6fdeaf20718ea3f8286fe041e/xarray/core/groupby.py#L422-L430
JoelJaeschke: Hi @dcherian, I implemented the feedback as you described, hope this does the trick.
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
```

Patch:
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index fe678e7b..b47af127 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -22,6 +22,8 @@ v2024.06.1 (unreleased)
 
 New Features
 ~~~~~~~~~~~~
+- Use fastpath when grouping both montonically increasing and decreasing variable
+  in :py:class:`GroupBy` (:issue:`6220`, :pull:`7427`). By `Joel Jaeschke <https://github.com/joeljaeschke>`_.
 - Introduce new :py:class:`groupers.UniqueGrouper`, :py:class:`groupers.BinGrouper`, and
   :py:class:`groupers.TimeResampler` objects as a step towards supporting grouping by
   multiple variables. See the `docs <groupby.groupers_>` and the
diff --git a/xarray/core/groupers.py b/xarray/core/groupers.py
index 4ee500cf..1b96988b 100644
--- a/xarray/core/groupers.py
+++ b/xarray/core/groupers.py
@@ -123,7 +123,9 @@ class UniqueGrouper(Grouper):
         if isinstance(self.group, _DummyGroup):
             return True
         index = self.group_as_index
-        return index.is_unique and index.is_monotonic_increasing
+        return index.is_unique and (
+            index.is_monotonic_increasing or index.is_monotonic_decreasing
+        )
 
     @property
     def group_as_index(self) -> pd.Index:
@@ -326,7 +328,7 @@ class TimeResampler(Resampler):
 
         if not group_as_index.is_monotonic_increasing:
             # TODO: sort instead of raising an error
-            raise ValueError("index must be monotonic for resampling")
+            raise ValueError("Index must be monotonic for resampling")
 
         if isinstance(group_as_index, CFTimeIndex):
             from xarray.core.resample_cftime import CFTimeGrouper


Test patch:
diff --git a/xarray/tests/test_groupby.py b/xarray/tests/test_groupby.py
index 2f169079..9519a555 100644
--- a/xarray/tests/test_groupby.py
+++ b/xarray/tests/test_groupby.py
@@ -1792,6 +1792,23 @@ class TestDataArrayGroupBy:
         actual = a.groupby("b").fillna(DataArray([0, 2], dims="b"))
         assert_identical(expected, actual)
 
+    @pytest.mark.parametrize("use_flox", [True, False])
+    def test_groupby_fastpath_for_monotonic(self, use_flox: bool) -> None:
+        # Fixes https://github.com/pydata/xarray/issues/6220
+        # Fixes https://github.com/pydata/xarray/issues/9279
+        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])
+
+        with xr.set_options(use_flox=use_flox):
+            assert_identical(fwd.sum(), array)
+            assert_identical(rev.sum(), array_rev)
+
 
 class TestDataArrayResample:
     @pytest.mark.parametrize("use_cftime", [True, False])
@@ -1828,9 +1845,13 @@ class TestDataArrayResample:
         expected = resample_as_pandas(array, "24h", closed="right")
         assert_identical(expected, actual)
 
-        with pytest.raises(ValueError, match=r"index must be monotonic"):
+        with pytest.raises(ValueError, match=r"Index must be monotonic"):
             array[[2, 0, 1]].resample(time="1D")
 
+        reverse = array.isel(time=slice(-1, None, -1))
+        with pytest.raises(ValueError):
+            reverse.resample(time="1D").mean()
+
     @pytest.mark.parametrize("use_cftime", [True, False])
     def test_resample_doctest(self, use_cftime: bool) -> None:
         # run the doctest example here so we are not surprised