From 58ab6eedfb0833150b6862ed1487b660c57a9b8b Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sun, 26 Jul 2026 20:07:16 +0200 Subject: [PATCH 1/3] TST: cover the CoordRange fast paths missed on dev The perf work in #778 left two lines uncovered in dascore/core/coords.py: - the ndarray branch of _round_ratio in the CoordRange validator, which is dead code (multi-element arrays are rejected earlier by the pd.isnull check, so the ratio is always scalar-like); it is removed. - the guard returning None when the index fraction isn't finite, i.e. a CoordRange with a step of 0. Selecting on such a coord with python (not numpy) scalars raised ZeroDivisionError rather than returning everything, which the array-based implementation prior to #778 did; that is fixed and both cases are now tested. --- dascore/core/coords.py | 18 +++++++++++++----- tests/test_core/test_coords.py | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index e2b338506..0c26bb141 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1392,10 +1392,10 @@ def _maybe_unbox_scalar(value): def _round_ratio(numerator, denominator, digits): """Round numerator/denominator, cheaply for scalars.""" + # Inputs are always scalar-like (multi-element arrays are rejected + # by the pd.isnull check above) and rounding python floats is + # ~10x faster than numpy scalars, hence the float conversion. ratio = _maybe_unbox_scalar(numerator / denominator) - if isinstance(ratio, np.ndarray): # multi-element array inputs - return np.round(ratio, digits) - # rounding python floats is ~10x faster than numpy scalars. return round(float(ratio), digits) zero = _TD64_ZERO if is_timedelta64(step) else 0 @@ -1529,8 +1529,16 @@ def _get_index(self, value, forward=True): # Scalar fast path; avoids several small-array allocations. # Due to float weirdness we need a little bit of a fudge factor. # (float() first since rounding numpy scalars is ~10x slower) - fraction = round(float((value - start) / step), 10) - if not math.isfinite(fraction): # e.g. a step of 0 + # A step of 0 (a len 1 CoordRange) has no index to compute, so + # None is returned, meaning the value doesn't constrain the + # selection. It is caught after the division (python scalars + # raise, numpy scalars give inf/nan) since testing a numpy step + # for truthiness up front costs ~10x more on this hot path. + try: + fraction = round(float((value - start) / step), 10) + except ZeroDivisionError: + return None + if not math.isfinite(fraction): return None out = math.ceil(fraction) if forward else math.floor(fraction) if forward and out < 0: diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index 9256424e3..7aaa27377 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -1369,6 +1369,29 @@ def test_coord_range_len_1(self): assert len(new) == 1 assert np.all(new.values == np.zeros(0)) + @pytest.mark.parametrize( + "start,step", + [ + (0, 0), + (np.float64(0), np.float64(0)), + (np.datetime64("2020-01-01", "ns"), np.timedelta64(0, "ns")), + ], + ) + def test_select_step_of_0(self, start, step): + """A step of 0 has no index to select on, so all values are kept.""" + coord = CoordRange(start=start, stop=start, step=step) + # numpy scalars warn (rather than raise) on the zero division. + with np.errstate(divide="ignore", invalid="ignore"): + new, index = coord.select((start, start + 10 * (step + 1))) + assert new == coord + assert index == slice(None, None, None) + + 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)) + assert len(coord) == 10 + assert coord.step == 1.0 + def test_reversed_coord(self, evenly_sampled_reversed_coord): """Ensure reverse sampling works for evenly sampled coord.""" coord = evenly_sampled_reversed_coord From 24ef1576cbe402b9bb7345030332ed2748cb67fc Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sun, 26 Jul 2026 20:19:53 +0200 Subject: [PATCH 2/3] Give a step of 0 CoordRange proper select semantics Every sample of such a coord equals start, so bounds which don't contain start now yield a degenerate selection instead of keeping the sample. --- dascore/core/coords.py | 25 ++++++++++++++++++------- tests/test_core/test_coords.py | 24 +++++++++++++++--------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 0c26bb141..fa76dcc3e 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1520,6 +1520,19 @@ def sort(self, reverse=False) -> tuple[BaseCoord, slice | ArrayLike]: out = self._new_grid(new_start, new_step, len(self)) return out, slice(None, None, -1) + def _get_zero_step_index(self, value, forward): + """ + Get the index of a value for a coord with a step of 0. + + Every sample of such a coord equals start, so the index is either the + first sample or one just outside the coord, which makes the + selection degenerate. + """ + start = self.start + if forward: # index of the first sample >= value + return 0 if value <= start else len(self) + return 0 if value >= start else -1 + def _get_index(self, value, forward=True): """Get the index corresponding to a value.""" if (value := self._get_compatible_value(value)) is None: @@ -1529,17 +1542,15 @@ def _get_index(self, value, forward=True): # Scalar fast path; avoids several small-array allocations. # Due to float weirdness we need a little bit of a fudge factor. # (float() first since rounding numpy scalars is ~10x slower) - # A step of 0 (a len 1 CoordRange) has no index to compute, so - # None is returned, meaning the value doesn't constrain the - # selection. It is caught after the division (python scalars - # raise, numpy scalars give inf/nan) since testing a numpy step - # for truthiness up front costs ~10x more on this hot path. + # A step of 0 is handled after the division (python scalars raise, + # numpy scalars give inf/nan) since testing a numpy step for + # truthiness up front costs ~10x more on this hot path. try: fraction = round(float((value - start) / step), 10) except ZeroDivisionError: - return None + return self._get_zero_step_index(value, forward) if not math.isfinite(fraction): - return None + return self._get_zero_step_index(value, forward) 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 7aaa27377..fee555613 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -1370,21 +1370,27 @@ def test_coord_range_len_1(self): assert np.all(new.values == np.zeros(0)) @pytest.mark.parametrize( - "start,step", + "start,unit", [ - (0, 0), - (np.float64(0), np.float64(0)), - (np.datetime64("2020-01-01", "ns"), np.timedelta64(0, "ns")), + (0, 1), + (np.float64(0), np.float64(1)), + (np.datetime64("2020-01-01", "ns"), np.timedelta64(1, "s")), ], ) - def test_select_step_of_0(self, start, step): - """A step of 0 has no index to select on, so all values are kept.""" + def test_select_step_of_0(self, start, unit): + """All samples of a step of 0 coord equal start; select on that.""" + step = start - start # a zero step of the right type. coord = CoordRange(start=start, stop=start, step=step) # numpy scalars warn (rather than raise) on the zero division. with np.errstate(divide="ignore", invalid="ignore"): - new, index = coord.select((start, start + 10 * (step + 1))) - assert new == coord - assert index == slice(None, None, None) + # a range which contains start keeps the sample. + kept, index = coord.select((start - unit, start + unit)) + # ranges which don't are degenerate. + after, _ = coord.select((start + unit, start + 2 * unit)) + before, _ = coord.select((start - 2 * unit, start - unit)) + assert kept == coord + assert index == slice(None, 1, None) + assert after.degenerate and before.degenerate def test_zero_dim_array_inputs(self): """Ensure 0d arrays (which aren't unboxed) can init a CoordRange.""" From 6cf424b76ae3cf7af3bce8ad71acdb1cdbea9296 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sun, 26 Jul 2026 20:43:03 +0200 Subject: [PATCH 3/3] Mark the remote cache and resource manager thread tests They spawn threads via run_in_threads, which fails in WebAssembly where the wasm suite deselects concurrency tests. --- tests/test_utils/test_io_utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_utils/test_io_utils.py b/tests/test_utils/test_io_utils.py index a2a353ee9..8c1fcf3f5 100644 --- a/tests/test_utils/test_io_utils.py +++ b/tests/test_utils/test_io_utils.py @@ -1240,6 +1240,7 @@ def _memory_file(self, name: str) -> UPath: fi.write(b"dascore" * 64) return path + @pytest.mark.concurrency def test_racing_callers_download_once(self, monkeypatch, run_in_threads): """Callers wanting one resource agree on the path and download it once.""" resource = self._memory_file("shared.bin") @@ -1256,6 +1257,7 @@ def _counted(path, local_path): assert len(downloads) == 1 assert results[0].exists() + @pytest.mark.concurrency def test_distinct_resources_are_not_serialized(self, monkeypatch, run_in_threads): """Unrelated downloads run at once; one global lock would time out here.""" resources = [self._memory_file(f"file_{i}.bin") for i in range(4)] @@ -1311,6 +1313,7 @@ def test_reinit_drops_download_locks(self): class TestIOResourceManagerConcurrency: """One manager hands every caller the same handle per type.""" + @pytest.mark.concurrency def test_racing_callers_share_one_handle(self, tmp_path, run_in_threads): """get_resource opens each required type exactly once.""" path = tmp_path / "concurrent_resource.bin"