Skip to content

Commit c35d6b6

Browse files
authored
FIX: reverse index output of bottleneck move_argmax/move_argmin functions (pydata#8552)
* FIX: reverse index output of bottleneck move_argmax/move_argmin functions, add move_argmax/move_argmin to bottleneck tests * add whats-new.rst entry
1 parent b444438 commit c35d6b6

File tree

3 files changed

+18
-2
lines changed

3 files changed

+18
-2
lines changed

doc/whats-new.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ Deprecations
3838
Bug fixes
3939
~~~~~~~~~
4040

41+
- Reverse index output of bottleneck's rolling move_argmax/move_argmin functions (:issue:`8541`, :pull:`8552`).
42+
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
43+
4144

4245
Documentation
4346
~~~~~~~~~~~~~

xarray/core/rolling.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,11 @@ def _bottleneck_reduce(self, func, keep_attrs, **kwargs):
596596
values = func(
597597
padded.data, window=self.window[0], min_count=min_count, axis=axis
598598
)
599+
# index 0 is at the rightmost edge of the window
600+
# need to reverse index here
601+
# see GH #8541
602+
if func in [bottleneck.move_argmin, bottleneck.move_argmax]:
603+
values = self.window[0] - 1 - values
599604

600605
if self.center[0]:
601606
values = values[valid]

xarray/tests/test_rolling.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ def test_rolling_properties(self, da) -> None:
120120
):
121121
da.rolling(foo=2)
122122

123-
@pytest.mark.parametrize("name", ("sum", "mean", "std", "min", "max", "median"))
123+
@pytest.mark.parametrize(
124+
"name", ("sum", "mean", "std", "min", "max", "median", "argmin", "argmax")
125+
)
124126
@pytest.mark.parametrize("center", (True, False, None))
125127
@pytest.mark.parametrize("min_periods", (1, None))
126128
@pytest.mark.parametrize("backend", ["numpy"], indirect=True)
@@ -133,9 +135,15 @@ def test_rolling_wrapped_bottleneck(
133135

134136
func_name = f"move_{name}"
135137
actual = getattr(rolling_obj, name)()
138+
window = 7
136139
expected = getattr(bn, func_name)(
137-
da.values, window=7, axis=1, min_count=min_periods
140+
da.values, window=window, axis=1, min_count=min_periods
138141
)
142+
# index 0 is at the rightmost edge of the window
143+
# need to reverse index here
144+
# see GH #8541
145+
if func_name in ["move_argmin", "move_argmax"]:
146+
expected = window - 1 - expected
139147

140148
# Using assert_allclose because we get tiny (1e-17) differences in numbagg.
141149
np.testing.assert_allclose(actual.values, expected)

0 commit comments

Comments
 (0)