From fe31a552bed2d4848420dde7b39f02385edeef44 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 13 Aug 2026 07:09:34 +0200 Subject: [PATCH 1/3] Treat a non-finite select bound as an open side `CoordRange._get_index` divides by the step and, when the result is not finite, took that as a zero step. Two things reach that branch: a zero step, whose samples all equal start, and a non-finite bound, which has no index because it means an open side. Reading the second as the first produced a backwards slice, so `patch.select(distance=(50, np.inf))` raised `ValueError: __len__() should return >= 0`. Against the last release the same call silently returned an empty patch: v0.1.20 resolved the bound through `np.floor(inf).astype(np.int64)`, which is `INT64_MIN`, and clamped to nothing. Tested at both levels, since the branch was already exercised by the zero step case it was written for and the regression still got through. --- dascore/core/coords.py | 7 ++++++- tests/test_core/test_coords.py | 9 +++++++++ tests/test_proc/test_proc_coords.py | 7 +++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 8144bd350..82223661c 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1651,7 +1651,12 @@ def _get_index(self, value, forward=True): except ZeroDivisionError: return self._get_zero_step_index(value, forward) if not math.isfinite(fraction): - return self._get_zero_step_index(value, forward) + # Two things land here. A zero step, whose samples all equal + # start, has a degenerate but defined index. A non-finite + # bound (np.inf, NaN) has none; it means an open side. + if step == 0: + return self._get_zero_step_index(value, forward) + return None out = math.ceil(fraction) if forward else math.floor(fraction) if forward and out < 0: return None diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index f8e2ec728..d18bac630 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -1438,6 +1438,15 @@ def test_select_step_of_0(self, start, unit): assert index == slice(None, 1, None) assert after.degenerate and before.degenerate + @pytest.mark.parametrize("bound", [np.inf, np.nan]) + def test_select_non_finite_bound(self, bound): + """A non-finite bound means an open side, not a zero-step index.""" + coord = get_coord(start=0, stop=100, step=1) + for value in (coord.min(), coord.max()): + assert coord.select((value, bound)) == coord.select((value, ...)) + assert coord.select((-bound, value)) == coord.select((..., value)) + assert coord.select((-bound, bound)) == coord.select((..., ...)) + def test_zero_dim_array_inputs(self): """Ensure 0d arrays (which aren't unboxed) can init a CoordRange.""" coord = CoordRange(start=np.array(0.0), stop=np.array(10.0), step=np.array(1.0)) diff --git a/tests/test_proc/test_proc_coords.py b/tests/test_proc/test_proc_coords.py index deff814e7..9b4b29502 100644 --- a/tests/test_proc/test_proc_coords.py +++ b/tests/test_proc/test_proc_coords.py @@ -292,6 +292,13 @@ def _assert_data_shape_unchanged(self, original_patch, selected_patch): assert selected_patch.data.shape == original_patch.data.shape assert np.array_equal(selected_patch.data, original_patch.data) + def test_select_infinite_bound(self, random_patch): + """An infinite bound is an open one, like ... or None.""" + dmin = random_patch.get_coord("distance").min() + expected = random_patch.select(distance=(dmin, ...)) + assert random_patch.select(distance=(dmin, np.inf)).equals(expected) + assert random_patch.select(distance=(-np.inf, ...)).equals(expected) + def test_select_by_distance(self, random_patch): """Ensure distance can be used to filter patch.""" dmin, dmax = 100, 200 From 4ba52c6f3cf38e21b3236bdcbb1758a9f540ae82 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 13 Aug 2026 07:37:26 +0200 Subject: [PATCH 2/3] Tell an infinite bound from one which overflows the division Reviewers pointed out that a finite bound can also make the fraction non-finite: `(value - start) / step` overflows when the two are far enough apart, and reading that as an open side selects everything when the bound asked for nothing. The sign settles both cases without asking where the infinity came from. An infinite fraction is a bound past one end of the coord, and which end it is decides the answer: an upper bound above every sample is the same as no upper bound, while a lower bound above every sample selects nothing. That is what the range checks below already say, so the branch only has to name the end. NaN keeps its own line, since it names no end at all. Also unbox a 0d array, which is Sized and so took the array path, where an infinite bound was cast to the smallest int64 and the selection came back empty. --- dascore/constants.py | 3 ++- dascore/core/coords.py | 26 ++++++++++++++++++++------ docs/tutorial/patch.qmd | 2 +- tests/test_core/test_coords.py | 22 +++++++++++++++++++++- tests/test_proc/test_proc_coords.py | 11 +++++++---- 5 files changed, 51 insertions(+), 13 deletions(-) diff --git a/dascore/constants.py b/dascore/constants.py index de3f97b99..ed171435d 100644 --- a/dascore/constants.py +++ b/dascore/constants.py @@ -235,7 +235,8 @@ def map(self, fn: Callable, iterable: Iterable, /) -> Iterable: select_values_description = """ Any dimension name can be passed as key, and the values can be: - a tuple of (min, max) for that dimension, or an equivalent slice. - `None` and ... both indicate open intervals. + `None` and ... both indicate open intervals, as does an infinite + bound pointing away from the data, eg `(min, np.inf)`. - an integer, when `samples=True`, to select a single row or column. - an array of values to select, which must be a subset of the coordinate array. diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 82223661c..65d814e8c 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1651,18 +1651,32 @@ def _get_index(self, value, forward=True): except ZeroDivisionError: return self._get_zero_step_index(value, forward) if not math.isfinite(fraction): - # Two things land here. A zero step, whose samples all equal - # start, has a degenerate but defined index. A non-finite - # bound (np.inf, NaN) has none; it means an open side. - if step == 0: + # A zero step, whose samples all equal start, has a + # degenerate but defined index. Ask it by truthiness: a + # timedelta step compared to a bare 0 warns (and will + # eventually raise) about the generic unit that implies. + if not step: return self._get_zero_step_index(value, forward) - return None - out = math.ceil(fraction) if forward else math.floor(fraction) + # NaN names no position at all, so it is an open side. + if math.isnan(fraction): + return None + # An infinite fraction is a bound past one end of the coord, + # either because the bound itself is infinite or because it + # sits far enough from start to overflow the division. Its + # sign says which end, and the range checks below turn the + # end the samples are not on into an open side. + out = len(self) if fraction > 0 else -1 + else: + out = math.ceil(fraction) if forward else math.floor(fraction) if forward and out < 0: return None if not forward and out >= len(self): return None return out + # A 0d array is Sized but holds a single bound, so unbox it rather + # than let the array path cast its infinity to the smallest int64. + if getattr(value, "ndim", 1) == 0: + return self._get_index(value[()], forward=forward) array = np.atleast_1d(value) func = np.ceil if forward else np.floor # Due to float weirdness we need a little bit of a fudge factor here. diff --git a/docs/tutorial/patch.qmd b/docs/tutorial/patch.qmd index cf9eea52a..8670a14c4 100644 --- a/docs/tutorial/patch.qmd +++ b/docs/tutorial/patch.qmd @@ -339,7 +339,7 @@ The following methods help trim, reshape, and manipulate coordinates. ## Select -Patches are trimmed using the [`Patch.select`](`dascore.Patch.select`) method. Unlike [`Patch.order`](`dascore.Patch.order`), `select` will not change the order of the affected dimensions, it will only remove elements. Most commonly, `select` takes the coordinate name and a tuple of (lower_limit, upper_limit) as the values. Either limit can be `...` indicating an open interval. +Patches are trimmed using the [`Patch.select`](`dascore.Patch.select`) method. Unlike [`Patch.order`](`dascore.Patch.order`), `select` will not change the order of the affected dimensions, it will only remove elements. Most commonly, `select` takes the coordinate name and a tuple of (lower_limit, upper_limit) as the values. Either limit can be `...` or `None`, indicating an open interval, as can an infinite bound pointing away from the data, such as `(lower_limit, np.inf)`. ```{python} import numpy as np diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index d18bac630..f6638beee 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -1442,10 +1442,30 @@ def test_select_step_of_0(self, start, unit): def test_select_non_finite_bound(self, bound): """A non-finite bound means an open side, not a zero-step index.""" coord = get_coord(start=0, stop=100, step=1) - for value in (coord.min(), coord.max()): + for value in (coord.min(), 42, coord.max()): assert coord.select((value, bound)) == coord.select((value, ...)) assert coord.select((-bound, value)) == coord.select((..., value)) assert coord.select((-bound, bound)) == coord.select((..., ...)) + # The open side must not swallow the finite one; 42 really trims. + assert len(coord.select((42, bound))[0]) < len(coord) + + def test_select_infinite_bound_pointing_at_the_data(self): + """An infinite bound is open on the side it points, empty on the other.""" + coord = get_coord(start=0, stop=100, step=1) + # No sample is >= inf, nor <= -inf, so these ask for nothing. + assert not len(coord.select((np.inf, None))[0]) + assert not len(coord.select((None, -np.inf))[0]) + + def test_select_non_finite_bound_in_0d_array(self): + """A 0d array holds one bound, so it is open like the scalar is.""" + coord = get_coord(start=0, stop=100, step=1) + assert coord.select((42, np.array(np.inf))) == coord.select((42, ...)) + + def test_select_bound_which_overflows_the_division(self): + """A bound too far from start to divide is outside, not open.""" + coord = get_coord(start=0.0, stop=1e-305, step=1e-308) + assert not len(coord.select((1e308, None))[0]) + assert not len(coord.select((None, -1e308))[0]) def test_zero_dim_array_inputs(self): """Ensure 0d arrays (which aren't unboxed) can init a CoordRange.""" diff --git a/tests/test_proc/test_proc_coords.py b/tests/test_proc/test_proc_coords.py index 9b4b29502..4a01b6123 100644 --- a/tests/test_proc/test_proc_coords.py +++ b/tests/test_proc/test_proc_coords.py @@ -294,10 +294,13 @@ def _assert_data_shape_unchanged(self, original_patch, selected_patch): def test_select_infinite_bound(self, random_patch): """An infinite bound is an open one, like ... or None.""" - dmin = random_patch.get_coord("distance").min() - expected = random_patch.select(distance=(dmin, ...)) - assert random_patch.select(distance=(dmin, np.inf)).equals(expected) - assert random_patch.select(distance=(-np.inf, ...)).equals(expected) + coord = random_patch.get_coord("distance") + middle = coord.values[len(coord) // 2] + expected = random_patch.select(distance=(middle, ...)) + assert random_patch.select(distance=(middle, np.inf)).equals(expected) + assert random_patch.select(distance=(-np.inf, ...)).equals(random_patch) + # The infinite side must not swallow the finite one. + assert expected.shape != random_patch.shape def test_select_by_distance(self, random_patch): """Ensure distance can be used to filter patch.""" From 405d49874f8ebc598b9f66ccbf9483adee775df4 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 13 Aug 2026 08:00:44 +0200 Subject: [PATCH 3/3] Drop the NaN branch, which nothing can reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_get_compatible_value` reads a null bound — NaN and NaT alike — as None and returns before the division, so a NaN fraction only ever comes from a zero step, which the line above already answers. Codecov is what noticed: the branch was the one new line no test covered, and no test could. --- dascore/core/coords.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 65d814e8c..f2a0d7435 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1657,14 +1657,13 @@ def _get_index(self, value, forward=True): # eventually raise) about the generic unit that implies. if not step: return self._get_zero_step_index(value, forward) - # NaN names no position at all, so it is an open side. - if math.isnan(fraction): - return None - # An infinite fraction is a bound past one end of the coord, - # either because the bound itself is infinite or because it - # sits far enough from start to overflow the division. Its - # sign says which end, and the range checks below turn the - # end the samples are not on into an open side. + # Otherwise the fraction is infinite: a bound past one end of + # the coord, either because the bound itself is infinite or + # because it sits far enough from start to overflow the + # division. Its sign says which end, and the range checks + # below turn the end the samples are not on into an open + # side. It cannot be NaN; a null bound, NaN and NaT alike, + # is already None by the time it gets here. out = len(self) if fraction > 0 else -1 else: out = math.ceil(fraction) if forward else math.floor(fraction)