From 63c1b6addadbac07361fe0dbe6c6d9ede9c06b9c Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 14:31:14 -0400 Subject: [PATCH 01/15] sketch of solution --- xarray/core/indexes.py | 49 ++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index d22fc37aa4f..03ad3c97b7b 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -559,24 +559,39 @@ def _sanitize_slice_element(x): return x -def _query_slice(index, label, coord_name="", method=None, tolerance=None): +def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> slice: + slice_label_start = _sanitize_slice_element(label.start) + slice_label_stop = _sanitize_slice_element(label.stop) + slice_label_step = _sanitize_slice_element(label.step) + if method is not None or tolerance is not None: - raise NotImplementedError( - "cannot use ``method`` argument if any indexers are slice objects" - ) - indexer = index.slice_indexer( - _sanitize_slice_element(label.start), - _sanitize_slice_element(label.stop), - _sanitize_slice_element(label.step), - ) - if not isinstance(indexer, slice): - # unlike pandas, in xarray we never want to silently convert a - # slice indexer into an array indexer - raise KeyError( - "cannot represent labeled-based slice indexer for coordinate " - f"{coord_name!r} with a slice over integer positions; the index is " - "unsorted or non-unique" + # likely slower because it requires two lookups, but pandas.Index.slice_indexer doesn't support method or tolerance + slice_index_start = index.get_indexer([slice_label_start], method=method, tolerance=tolerance) + slice_index_stop = index.get_indexer([slice_label_stop], method=method, tolerance=tolerance) + + if slice_label_step not in [None, 1]: + # TODO test that passing this through works + raise NotImplementedError(f"unsure how to handle step = {slice_label_step}") + + # TODO handle start being greater than stop + # TODO handle non-zero step + # TODO is there already a function for this somewhere? + indexer = slice(slice_index_start.item(), slice_index_stop.item()) + else: + indexer = index.slice_indexer( + slice_label_start, + slice_label_stop, + slice_label_step, ) + if not isinstance(indexer, slice): + # unlike pandas, in xarray we never want to silently convert a + # slice indexer into an array indexer + raise KeyError( + "cannot represent labeled-based slice indexer for coordinate " + f"{coord_name!r} with a slice over integer positions; the index is " + "unsorted or non-unique" + ) + return indexer @@ -817,6 +832,8 @@ def isel( # scalar indexer: drop index return None + print(indxr) + return self._replace(self.index[indxr]) # type: ignore[index] def sel( From c03b0e94a70e8555b3e41df1c84e89c261bf90cd Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 15:21:29 -0400 Subject: [PATCH 02/15] initial test --- xarray/tests/test_dataset.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index e31b687ca77..7cbfae0f40a 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2175,6 +2175,27 @@ def test_sel_method(self) -> None: with pytest.raises(ValueError, match=r"cannot supply"): data.sel(dim1=0, method="nearest") + def test_sel_method_with_slice(self) -> None: + # regression test for https://github.com/pydata/xarray/issues/10710 + + data_int_coords = xr.Dataset(coords={"lat": ("lat", [20, 21, 22, 23])}) + expected = xr.Dataset(coords={"lat": ("lat", [21, 22])}) + actual = data_int_coords.sel(lat=slice(21, 22), method="nearest") + assert_identical(expected, actual) + + # check consistency with not passing method kwarg, for case of ints, where method kwarg should be irrelevant + expected = data_int_coords.sel(lat=slice(21, 22)) + assert_identical(expected, actual) + + data_float_coords = xr.Dataset( + coords={"lat": ("lat", [20.1, 21.1, 22.1, 23.1])} + ) + expected = xr.Dataset(coords={"lat": ("lat", [21.1, 22.1])}) + actual = data_float_coords.sel(lat=slice(21, 22), method="nearest") + assert_identical(expected, actual) + + # TODO backwards slices? + def test_loc(self) -> None: data = create_test_data() expected = data.sel(dim3="a") From e65b759e069bba7ccd4de3c41a6a16d7a5575966 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 15:21:38 -0400 Subject: [PATCH 03/15] test passing --- xarray/core/indexes.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 03ad3c97b7b..41ddba9b801 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -563,12 +563,17 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl slice_label_start = _sanitize_slice_element(label.start) slice_label_stop = _sanitize_slice_element(label.stop) slice_label_step = _sanitize_slice_element(label.step) - + if method is not None or tolerance is not None: # likely slower because it requires two lookups, but pandas.Index.slice_indexer doesn't support method or tolerance - slice_index_start = index.get_indexer([slice_label_start], method=method, tolerance=tolerance) - slice_index_stop = index.get_indexer([slice_label_stop], method=method, tolerance=tolerance) - + # see https://github.com/pydata/xarray/issues/10710 + slice_index_start = index.get_indexer( + [slice_label_start], method=method, tolerance=tolerance + ) + slice_index_stop = index.get_indexer( + [slice_label_stop], method=method, tolerance=tolerance + ) + if slice_label_step not in [None, 1]: # TODO test that passing this through works raise NotImplementedError(f"unsure how to handle step = {slice_label_step}") @@ -576,7 +581,8 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl # TODO handle start being greater than stop # TODO handle non-zero step # TODO is there already a function for this somewhere? - indexer = slice(slice_index_start.item(), slice_index_stop.item()) + # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label + indexer = slice(slice_index_start.item(), slice_index_stop.item() + 1) else: indexer = index.slice_indexer( slice_label_start, @@ -591,7 +597,7 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl f"{coord_name!r} with a slice over integer positions; the index is " "unsorted or non-unique" ) - + return indexer From 20a6d3b59b46901e1393ac8b187ccc1223f88970 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 15:26:59 -0400 Subject: [PATCH 04/15] update existing test --- xarray/tests/test_dataset.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 7cbfae0f40a..0a1ba9f110c 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2164,8 +2164,9 @@ def test_sel_method(self) -> None: actual = data.sel(dim2=[1.45], method="backfill") assert_identical(expected, actual) - with pytest.raises(NotImplementedError, match=r"slice objects"): - data.sel(dim2=slice(1, 3), method="ffill") + expected = data.isel(dim2=slice(2, 7)) + actual = data.sel(dim2=slice(1, 3), method="ffill") + assert_identical(expected, actual) with pytest.raises(TypeError, match=r"``method``"): # this should not pass silently From ee1da65a35862f8d8f27d4c5fdb9def107c5b157 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 15:32:54 -0400 Subject: [PATCH 05/15] support non-zero step --- xarray/core/indexes.py | 13 +++++-------- xarray/tests/test_dataset.py | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 41ddba9b801..662bc37004d 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -562,7 +562,7 @@ def _sanitize_slice_element(x): def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> slice: slice_label_start = _sanitize_slice_element(label.start) slice_label_stop = _sanitize_slice_element(label.stop) - slice_label_step = _sanitize_slice_element(label.step) + slice_index_step = _sanitize_slice_element(label.step) if method is not None or tolerance is not None: # likely slower because it requires two lookups, but pandas.Index.slice_indexer doesn't support method or tolerance @@ -574,20 +574,17 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl [slice_label_stop], method=method, tolerance=tolerance ) - if slice_label_step not in [None, 1]: - # TODO test that passing this through works - raise NotImplementedError(f"unsure how to handle step = {slice_label_step}") - # TODO handle start being greater than stop - # TODO handle non-zero step # TODO is there already a function for this somewhere? # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label - indexer = slice(slice_index_start.item(), slice_index_stop.item() + 1) + indexer = slice( + slice_index_start.item(), slice_index_stop.item() + 1, slice_index_step + ) else: indexer = index.slice_indexer( slice_label_start, slice_label_stop, - slice_label_step, + slice_index_step, ) if not isinstance(indexer, slice): # unlike pandas, in xarray we never want to silently convert a diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 0a1ba9f110c..4c2f35fd49b 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2168,6 +2168,10 @@ def test_sel_method(self) -> None: actual = data.sel(dim2=slice(1, 3), method="ffill") assert_identical(expected, actual) + expected = data.isel(dim2=slice(2, 7, 2)) + actual = data.sel(dim2=slice(1, 3, 2), method="ffill") + assert_identical(expected, actual) + with pytest.raises(TypeError, match=r"``method``"): # this should not pass silently data.sel(dim2=1, method=data) # type: ignore[arg-type] @@ -2184,8 +2188,14 @@ def test_sel_method_with_slice(self) -> None: actual = data_int_coords.sel(lat=slice(21, 22), method="nearest") assert_identical(expected, actual) + # check non-zero step + expected = xr.Dataset(coords={"lat": ("lat", [21])}) + actual = data_int_coords.sel(lat=slice(21, 22, 2), method="nearest") + assert_identical(expected, actual) + # check consistency with not passing method kwarg, for case of ints, where method kwarg should be irrelevant expected = data_int_coords.sel(lat=slice(21, 22)) + actual = data_int_coords.sel(lat=slice(21, 22), method="nearest") assert_identical(expected, actual) data_float_coords = xr.Dataset( @@ -2195,6 +2205,11 @@ def test_sel_method_with_slice(self) -> None: actual = data_float_coords.sel(lat=slice(21, 22), method="nearest") assert_identical(expected, actual) + # check non-zero step + expected = xr.Dataset(coords={"lat": ("lat", [21.1])}) + actual = data_float_coords.sel(lat=slice(21, 22, 2), method="nearest") + assert_identical(expected, actual) + # TODO backwards slices? def test_loc(self) -> None: From 308d8c936b202ad9832f584a21da21f40aff0ec8 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 15:53:00 -0400 Subject: [PATCH 06/15] convince myself that negative slices do work as they should --- xarray/core/indexes.py | 1 - xarray/tests/test_dataset.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 662bc37004d..8903169ef96 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -574,7 +574,6 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl [slice_label_stop], method=method, tolerance=tolerance ) - # TODO handle start being greater than stop # TODO is there already a function for this somewhere? # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label indexer = slice( diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 4c2f35fd49b..0ce31e948ed 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2210,7 +2210,35 @@ def test_sel_method_with_slice(self) -> None: actual = data_float_coords.sel(lat=slice(21, 22, 2), method="nearest") assert_identical(expected, actual) - # TODO backwards slices? + # backwards slices + data_int_coords = xr.Dataset(coords={"lat": ("lat", [23, 22, 21, 20])}) + expected = xr.Dataset(coords={"lat": ("lat", [22, 21])}) + actual = data_int_coords.sel(lat=slice(22, 21), method="nearest") + assert_identical(expected, actual) + + data_float_coords = xr.Dataset( + coords={"lat": ("lat", [23.1, 22.1, 21.1, 20.1])} + ) + expected = xr.Dataset(coords={"lat": ("lat", [22.1, 21.1])}) + actual = data_float_coords.sel(lat=slice(22, 21), method="nearest") + assert_identical(expected, actual) + + def test_sel_negative_slices(self) -> None: + data_int_coords = xr.Dataset(coords={"lat": ("lat", [-23, -22, -21, -20])}) + expected = xr.Dataset(coords={"lat": ("lat", [-22, -21])}) + actual = data_int_coords.sel(lat=slice(-22, -21)) + assert_identical(expected, actual) + + expected = xr.Dataset(coords={"lat": ("lat", [-22, -21])}) + actual = data_int_coords.sel(lat=slice(-22, -21), method="nearest") + assert_identical(expected, actual) + + data_float_coords = xr.Dataset( + coords={"lat": ("lat", [-23.1, -22.1, -21.1, -20.1])} + ) + expected = xr.Dataset(coords={"lat": ("lat", [-22.1, -21.1])}) + actual = data_float_coords.sel(lat=slice(-22, -21), method="nearest") + assert_identical(expected, actual) def test_loc(self) -> None: data = create_test_data() From 61c14123d72400356f4fb878c94aa7691126aa35 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 16:26:08 -0400 Subject: [PATCH 07/15] remove print --- xarray/core/indexes.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 8903169ef96..091d15f2a74 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -834,8 +834,6 @@ def isel( # scalar indexer: drop index return None - print(indxr) - return self._replace(self.index[indxr]) # type: ignore[index] def sel( From d7ec7469501740f3947fb8e32ad0bdafffb885f0 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 16:26:18 -0400 Subject: [PATCH 08/15] remove todo --- xarray/core/indexes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 091d15f2a74..ce8f666a0d2 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -574,7 +574,6 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> sl [slice_label_stop], method=method, tolerance=tolerance ) - # TODO is there already a function for this somewhere? # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label indexer = slice( slice_index_start.item(), slice_index_stop.item() + 1, slice_index_step From 8842b235b17f9abd745361f2cb9e4b862ba6ab87 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 16:30:41 -0400 Subject: [PATCH 09/15] whatsnew --- doc/whats-new.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index edf8a1aa492..f3d88378506 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -23,6 +23,9 @@ New Features - ``compute=False`` is now supported by :py:meth:`DataTree.to_netcdf` and :py:meth:`DataTree.to_zarr`. By `Stephan Hoyer `_. +- ``.sel`` operations now support the ``method`` and `tolerance` keyword arguments, + for the case of indexing with a slice. + By `Tom Nicholas `_. Breaking changes ~~~~~~~~~~~~~~~~ From 1fe2961ce1f7da58af2e004da0af033ded926413 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 16:31:23 -0400 Subject: [PATCH 10/15] lint --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index f3d88378506..0befaf05c86 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -23,7 +23,7 @@ New Features - ``compute=False`` is now supported by :py:meth:`DataTree.to_netcdf` and :py:meth:`DataTree.to_zarr`. By `Stephan Hoyer `_. -- ``.sel`` operations now support the ``method`` and `tolerance` keyword arguments, +- ``.sel`` operations now support the ``method`` and ``tolerance`` keyword arguments, for the case of indexing with a slice. By `Tom Nicholas `_. From 4dd2d63eec62ef784a218bea748b83646b609a09 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 6 Sep 2025 16:40:33 -0400 Subject: [PATCH 11/15] remove return type hint --- xarray/core/indexes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 8ad704de6de..6a95ea74912 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -559,7 +559,7 @@ def _sanitize_slice_element(x): return x -def _query_slice(index, label, coord_name="", method=None, tolerance=None) -> slice: +def _query_slice(index, label, coord_name="", method=None, tolerance=None): slice_label_start = _sanitize_slice_element(label.start) slice_label_stop = _sanitize_slice_element(label.stop) slice_index_step = _sanitize_slice_element(label.step) From f10a3a2c5ca5c92555e38373d536b1254536c70b Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sun, 7 Sep 2025 10:37:45 -0400 Subject: [PATCH 12/15] use a single get_indexer call --- xarray/core/indexes.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 6a95ea74912..4a2c575ea39 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -565,18 +565,16 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None): slice_index_step = _sanitize_slice_element(label.step) if method is not None or tolerance is not None: - # likely slower because it requires two lookups, but pandas.Index.slice_indexer doesn't support method or tolerance - # see https://github.com/pydata/xarray/issues/10710 - slice_index_start = index.get_indexer( - [slice_label_start], method=method, tolerance=tolerance - ) - slice_index_stop = index.get_indexer( - [slice_label_stop], method=method, tolerance=tolerance + # `pandas.Index.slice_indexer` doesn't support method or tolerance (see https://github.com/pydata/xarray/issues/10710) + slice_index_bounds = index.get_indexer( + [slice_label_start, slice_label_stop], method=method, tolerance=tolerance ) # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label indexer = slice( - slice_index_start.item(), slice_index_stop.item() + 1, slice_index_step + slice_index_bounds[0].item(), + slice_index_bounds[1].item() + 1, + slice_index_step, ) else: indexer = index.slice_indexer( From d05ab1a8b2bb43f4f7a722d57782c59f7c872554 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Mon, 8 Sep 2025 15:56:38 -0400 Subject: [PATCH 13/15] handle no match case --- xarray/core/indexes.py | 17 +++++++++++------ xarray/tests/test_dataset.py | 7 +++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 4a2c575ea39..1be8d4fdbb2 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -570,18 +570,23 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None): [slice_label_start, slice_label_stop], method=method, tolerance=tolerance ) - # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label - indexer = slice( - slice_index_bounds[0].item(), - slice_index_bounds[1].item() + 1, - slice_index_step, - ) + if -1 in slice_index_bounds: + # "no match" case - return empty slice + indexer = slice(0, 0) + else: + # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label + indexer = slice( + slice_index_bounds[0].item(), + slice_index_bounds[1].item() + 1, + slice_index_step, + ) else: indexer = index.slice_indexer( slice_label_start, slice_label_stop, slice_index_step, ) + if not isinstance(indexer, slice): # unlike pandas, in xarray we never want to silently convert a # slice indexer into an array indexer diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index d4ed016ab72..310bc3c27ac 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2205,6 +2205,13 @@ def test_sel_method_with_slice(self) -> None: actual = data_float_coords.sel(lat=slice(21, 22), method="nearest") assert_identical(expected, actual) + # "no match" case - should return zero-size slice + expected = xr.Dataset(coords={"lat": ("lat", [])}) + actual = data_float_coords.sel( + lat=slice(21.5, 21.6), method="nearest", tolerance=1e-3 + ) + assert_identical(expected, actual) + # check non-zero step expected = xr.Dataset(coords={"lat": ("lat", [21.1])}) actual = data_float_coords.sel(lat=slice(21, 22, 2), method="nearest") From 199765e119476a5020e6c8b423ce2fbb29f1ab85 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Mon, 8 Sep 2025 16:34:33 -0400 Subject: [PATCH 14/15] explicitly handle non-unique coordinate values --- xarray/core/indexes.py | 7 +++++++ xarray/tests/test_dataset.py | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 1be8d4fdbb2..c5b535309a3 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -566,6 +566,13 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None): if method is not None or tolerance is not None: # `pandas.Index.slice_indexer` doesn't support method or tolerance (see https://github.com/pydata/xarray/issues/10710) + + if index.has_duplicates: + # `pandas.Index.get_indexer` disallows this, see https://github.com/pydata/xarray/pull/10711#discussion_r2331297608 + raise NotImplementedError( + "cannot use ``method`` argument with a slice object as an indexer and an index with non-unique values" + ) + slice_index_bounds = index.get_indexer( [slice_label_start, slice_label_stop], method=method, tolerance=tolerance ) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 310bc3c27ac..45354512200 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2212,7 +2212,28 @@ def test_sel_method_with_slice(self) -> None: ) assert_identical(expected, actual) + # "no match" case - should return zero-size slice + expected = xr.Dataset(coords={"lat": ("lat", [])}) + actual = data_float_coords.sel( + lat=slice(21.5, 21.6), method="nearest", tolerance=1e-3 + ) + assert_identical(expected, actual) + + # non-unique coordinate values + data_non_unique = xr.Dataset( + coords={"lat": ("lat", [20.1, 21.1, 21.1, 22.1, 22.1, 23.1])} + ) + expected = xr.Dataset(coords={"lat": ("lat", [21.1, 21.1, 22.1, 22.1])}) + with pytest.raises( + NotImplementedError, + match="slice object as an indexer and an index with non-unique values", + ): + data_non_unique.sel(lat=slice(21.0, 22.2), method="nearest") + # check non-zero step + data_float_coords = xr.Dataset( + coords={"lat": ("lat", [20.1, 21.1, 22.1, 23.1])} + ) expected = xr.Dataset(coords={"lat": ("lat", [21.1])}) actual = data_float_coords.sel(lat=slice(21, 22, 2), method="nearest") assert_identical(expected, actual) From 8cce3316c2743def195680a6ad5728a529b04a54 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Mon, 8 Sep 2025 17:34:21 -0400 Subject: [PATCH 15/15] support passing tolerance but not method --- xarray/core/indexes.py | 27 ++++++++++++++++++++------- xarray/tests/test_dataset.py | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index c5b535309a3..71bf2c0de05 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -573,18 +573,31 @@ def _query_slice(index, label, coord_name="", method=None, tolerance=None): "cannot use ``method`` argument with a slice object as an indexer and an index with non-unique values" ) - slice_index_bounds = index.get_indexer( - [slice_label_start, slice_label_stop], method=method, tolerance=tolerance - ) + if method is None and tolerance is not None: + # copies default behaviour of slicing with no tolerance, which is to be exclusive at both ends + slice_index_start = index.get_indexer( + [slice_label_start], method="backfill", tolerance=tolerance + ) + slice_index_stop = index.get_indexer( + [slice_label_stop], method="pad", tolerance=tolerance + ) + else: + # minor optimization to only issue a single `.get_indexer` call to get both start and end + slice_index_start, slice_index_stop = index.get_indexer( + [slice_label_start, slice_label_stop], + method=method, + tolerance=tolerance, + ) - if -1 in slice_index_bounds: - # "no match" case - return empty slice + if -1 in [slice_index_start, slice_index_stop]: + # how pandas indicates the "no match" case - we return empty slice indexer = slice(0, 0) else: # +1 needed to emulate behaviour of xarray sel with slice without method kwarg, which is inclusive of point at stop label + # assumes no duplicates, but we have forbidden that case above indexer = slice( - slice_index_bounds[0].item(), - slice_index_bounds[1].item() + 1, + slice_index_start.item(), + slice_index_stop.item() + 1, slice_index_step, ) else: diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 45354512200..1cff0678530 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2212,6 +2212,22 @@ def test_sel_method_with_slice(self) -> None: ) assert_identical(expected, actual) + # test supposed default behaviour + expected = xr.Dataset(coords={"lat": ("lat", [21.1, 22.1])}) + actual = data_float_coords.sel(lat=slice(21.0, 22.2)) + assert_identical(expected, actual) + + # tolerance specified but method not specified + expected = xr.Dataset(coords={"lat": ("lat", [21.1, 22.1])}) + actual = data_float_coords.sel( + lat=slice(21.0, 22.2), + tolerance=1.0, + ) + assert_identical(expected, actual) + # test this matches default behaviour without tolerance specified + default = data_float_coords.sel(lat=slice(21.0, 22.2)) + assert_identical(default, actual) + # "no match" case - should return zero-size slice expected = xr.Dataset(coords={"lat": ("lat", [])}) actual = data_float_coords.sel(