From 8ab6191c33000066f11db8ce0970cf29cf656cab Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Fri, 7 Aug 2026 13:13:49 +0200 Subject: [PATCH 1/2] Handle pint quantities explicitly in to_float to_float rejected every dimensional quantity -- including the seconds quantity its own docstring implies should work -- while silently accepting every dimensionless one. The silent path is the dangerous half: pint's __float__ converts a dimensionless quantity to base units, and information's base unit is the bit, so to_float(25 * MB) returned 200000000.0 rather than raising. Register Quantity so a time quantity converts to its duration in seconds and everything else raises UnitError naming the explicit alternatives. This matches the function's actual domain: every existing overload converts datetime64 to seconds since epoch, timedelta64 to seconds, or passes a plain number through. The magnitude is recursed back through to_float so an array-valued quantity takes the array path and an integer magnitude still widens to float. Instrumenting the full test suite found exactly one Quantity reaching to_float, in a test asserting the bits behavior, so nothing depended on the old semantics. Also raise UnitError with an explanatory message when filtering a unitless coordinate with a unit-bearing value, rather than letting pint's DimensionalityError leak out of proc.filter. --- dascore/proc/filter.py | 9 ++++++++- dascore/utils/time.py | 30 ++++++++++++++++++++++++++++- docs/changelog.qmd | 1 + tests/test_proc/test_filter.py | 6 ++++++ tests/test_utils/test_time.py | 35 +++++++++++++++++++++++++++++++++- 5 files changed, 78 insertions(+), 3 deletions(-) diff --git a/dascore/proc/filter.py b/dascore/proc/filter.py index 85b3878cf..bccf16051 100644 --- a/dascore/proc/filter.py +++ b/dascore/proc/filter.py @@ -319,7 +319,14 @@ def notch_filter(patch: PatchType, q: float, **kwargs) -> PatchType: for dim, axis, value in dinfo: coord = patch.get_coord(dim) # Invert units if needed - if isinstance(value, dc.units.Quantity) and coord.units is not None: + if isinstance(value, dc.units.Quantity): + if coord.units is None: + msg = ( + f"Cannot filter {dim!r} with {value}: the coordinate " + "has no units, so a unit-bearing value cannot be " + "interpreted. Pass a plain number instead." + ) + raise UnitError(msg) value, _ = get_inverted_quant(value, coord.units) # Check valid parameters w0 = to_float(value) diff --git a/dascore/utils/time.py b/dascore/utils/time.py index 7bae6fbe5..2f3d2ec2a 100644 --- a/dascore/utils/time.py +++ b/dascore/utils/time.py @@ -8,13 +8,15 @@ import numpy as np import pandas as pd +import pint +from pint import DimensionalityError from dascore.constants import ( NUMPY_TIME_UNIT_MAPPING, ONE_SECOND, timeable_types, ) -from dascore.exceptions import TimeError +from dascore.exceptions import TimeError, UnitError _NAT_DATETIME64 = np.datetime64("NaT", "ns") _NAT_TIMEDELTA64 = np.timedelta64("NaT", "ns") @@ -393,6 +395,32 @@ def to_float(obj: timeable_types | np.ndarray) -> np.ndarray: return float(obj) +@to_float.register(pint.Quantity) +def _quantity_to_float(quant: pint.Quantity) -> float: + """ + Convert a time quantity to seconds. + + Anything else raises: this function's output is a duration in + seconds, so there is no meaningful float for a length or a data + size. Without this, pint's `__float__` would silently convert any + *dimensionless* quantity to its base units — returning 2e8 for + `25 * MB`, whose base unit is the bit — while rejecting the time + quantities this function is actually for. + """ + try: + # recurse so an array-valued quantity takes the array path and a + # scalar one is always widened to float (a magnitude may be int) + return to_float(quant.to("s").magnitude) + except DimensionalityError: + msg = ( + f"Cannot convert {quant} to a float; only time quantities " + "have a float representation here (seconds). Convert " + "explicitly instead, eg dascore.units.convert_units or, for " + "data sizes, dascore.units.get_byte_count." + ) + raise UnitError(msg) from None + + @to_float.register(np.ndarray) @to_float.register(list) @to_float.register(tuple) diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 6b4d909cb..81958c4d7 100644 --- a/docs/changelog.qmd +++ b/docs/changelog.qmd @@ -4,6 +4,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f ## Unreleased API Changes +- **`to_float` handles pint quantities explicitly.** A time quantity now converts to its duration in seconds (`to_float(2 * dc.units.min) == 120.0`), which previously raised `DimensionalityError`. Every other quantity raises `UnitError` instead of being silently converted: pint's `float()` reduces a *dimensionless* quantity to base units, so a data size came back eight times too large (`25 MB` → `2e8`, the count in bits). Use `convert_units` or `get_byte_count` for an explicit conversion. Relatedly, filtering a unitless coordinate with a unit-bearing value (`patch.notch_filter(distance=5 * dc.units.m)` where distance has no units) now raises `UnitError` with an explanatory message rather than leaking pint's `DimensionalityError`. - `Patch.drop_coords` and `CoordManager.drop_coords` accept a sequence of names as well as bare names, so `patch.drop_coords(["latitude", "longitude"])` works alongside `patch.drop_coords("latitude", "longitude")`. Previously a list or set raised `TypeError` and a tuple or generator was silently ignored; a tuple now drops the named coordinates, and one naming a dimension raises `ParameterError` as a bare dimension name always has. - **Runtime configuration is now two-tier.** `dc.set_config(...)` sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. Temporary, thread/task-local overrides use the new `dc.config_context(...)` context manager, which restores on exit and isolates concurrent overrides via a `ContextVar`. `Spool.map(...)` captures the config active at the call and re-applies it in each worker, so thread- and process-pool workers observe the caller's config regardless of the pool's start method. Unknown config field names now raise instead of being silently ignored. Migrate `with dc.set_config(...)` to `with dc.config_context(...)`. - PRODML 2.1 now supports writing one raw time-by-distance patch with `dc.write` as a standalone HDF5 file. Full PRODML 2.3 conformance requires EPC/XML packaging, which DASCore does not write. diff --git a/tests/test_proc/test_filter.py b/tests/test_proc/test_filter.py index 35850002b..664fc8e73 100644 --- a/tests/test_proc/test_filter.py +++ b/tests/test_proc/test_filter.py @@ -302,6 +302,12 @@ def test_notch_filter_distance_units(self, random_patch): assert isinstance(filtered_patch, dc.Patch) assert not np.any(np.isnan(filtered_patch.data)) + def test_unitless_coord_with_quantity_raises(self, random_patch): + """A unit-bearing value needs a coordinate with units.""" + patch = random_patch.set_units(distance=None) + with pytest.raises(UnitError, match="has no units"): + patch.notch_filter(distance=5 * dc.units.m, q=30) + class TestSavgolFilter: """Simple tests on Savgol filter.""" diff --git a/tests/test_utils/test_time.py b/tests/test_utils/test_time.py index 255c66fb6..127dbbbcd 100644 --- a/tests/test_utils/test_time.py +++ b/tests/test_utils/test_time.py @@ -11,7 +11,7 @@ import dascore as dc from dascore.compat import random_state -from dascore.exceptions import TimeError +from dascore.exceptions import TimeError, UnitError from dascore.utils.time import ( is_datetime64, is_timedelta64, @@ -684,6 +684,39 @@ def test_series(self): assert isinstance(out1, pd.Series) assert isinstance(out2, pd.Series) + def test_time_quantity(self): + """A time quantity converts to its duration in seconds.""" + assert to_float(dc.get_quantity("2 s")) == 2.0 + assert to_float(dc.get_quantity("2 min")) == 120.0 + assert to_float(dc.get_quantity("500 ms")) == 0.5 + + def test_time_quantity_array(self): + """An array-valued time quantity keeps its shape.""" + quant = np.array([1.0, 2.0]) * dc.get_quantity("min") + out = to_float(quant) + assert np.allclose(out, [60.0, 120.0]) + + def test_time_quantity_returns_float(self): + """An integer magnitude is still widened to float.""" + assert isinstance(to_float(dc.get_quantity("2 s")), float) + + @pytest.mark.parametrize("value", ("10 m", "25 MB", "50%", "1 strain", "5", "1 Hz")) + def test_non_time_quantity_raises(self, value): + """Only time quantities have a float representation.""" + with pytest.raises(UnitError, match="only time quantities"): + to_float(dc.get_quantity(value)) + + def test_data_size_is_not_silently_converted(self): + """ + Guard the bits trap. + + pint's `__float__` converts a dimensionless quantity to base + units, and information's base unit is the bit, so bytes used to + come back eight times too large instead of raising. + """ + with pytest.raises(UnitError): + to_float(dc.get_quantity("25 MB")) + class TestIsTimeDelta: """Test suite for determining time deltas.""" From 4b688e8ca395cb473fa51ec7f3621544ef0aa0c7 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Fri, 7 Aug 2026 13:52:03 +0200 Subject: [PATCH 2/2] Widen the unitless-coordinate filter guard to all quantities Review found the guard's message claimed the value was "unit-bearing", which is wrong for a dimensionless quantity. Every quantity is rejected, including dimensionless ones: there is nothing to convert against, and the previous behavior read `20 %` as 0.2 Hz. Say that, test all three kinds, and describe the real scope in the changelog. Also surface the quantity contract on to_float's public docstring; the overload's own docstring is not rendered in the API docs. --- dascore/proc/filter.py | 7 +++++-- dascore/utils/time.py | 3 +++ docs/changelog.qmd | 2 +- tests/test_proc/test_filter.py | 16 +++++++++++++--- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/dascore/proc/filter.py b/dascore/proc/filter.py index bccf16051..42c9fa591 100644 --- a/dascore/proc/filter.py +++ b/dascore/proc/filter.py @@ -321,10 +321,13 @@ def notch_filter(patch: PatchType, q: float, **kwargs) -> PatchType: # Invert units if needed if isinstance(value, dc.units.Quantity): if coord.units is None: + # every quantity is rejected, dimensionless ones included: + # there is nothing to convert against, and reading `20 %` + # as 0.2 Hz (which is what used to happen) is a trap. msg = ( f"Cannot filter {dim!r} with {value}: the coordinate " - "has no units, so a unit-bearing value cannot be " - "interpreted. Pass a plain number instead." + "has no units, so a quantity cannot be interpreted " + "against it. Pass a plain number instead." ) raise UnitError(msg) value, _ = get_inverted_quant(value, coord.units) diff --git a/dascore/utils/time.py b/dascore/utils/time.py index 2f3d2ec2a..d07ae05ce 100644 --- a/dascore/utils/time.py +++ b/dascore/utils/time.py @@ -391,6 +391,9 @@ def to_float(obj: timeable_types | np.ndarray) -> np.ndarray: Convert various datetime/timedelta things to a float. Time offsets represent seconds, and datetimes are seconds from 1970. + A pint quantity of time is converted to seconds as well; any other + quantity raises [`UnitError`](`dascore.exceptions.UnitError`), since + a length or a data size has no float representation here. """ return float(obj) diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 81958c4d7..9ce98e446 100644 --- a/docs/changelog.qmd +++ b/docs/changelog.qmd @@ -4,7 +4,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f ## Unreleased API Changes -- **`to_float` handles pint quantities explicitly.** A time quantity now converts to its duration in seconds (`to_float(2 * dc.units.min) == 120.0`), which previously raised `DimensionalityError`. Every other quantity raises `UnitError` instead of being silently converted: pint's `float()` reduces a *dimensionless* quantity to base units, so a data size came back eight times too large (`25 MB` → `2e8`, the count in bits). Use `convert_units` or `get_byte_count` for an explicit conversion. Relatedly, filtering a unitless coordinate with a unit-bearing value (`patch.notch_filter(distance=5 * dc.units.m)` where distance has no units) now raises `UnitError` with an explanatory message rather than leaking pint's `DimensionalityError`. +- **`to_float` handles pint quantities explicitly.** A time quantity now converts to its duration in seconds (`to_float(2 * dc.units.min) == 120.0`), which previously raised `DimensionalityError`. Every other quantity raises `UnitError` instead of being silently converted: pint's `float()` reduces a *dimensionless* quantity to base units, so a data size came back eight times too large (`25 MB` → `2e8`, the count in bits). Use `convert_units` or `get_byte_count` for an explicit conversion. Relatedly, filtering a coordinate that has no units with *any* quantity — `patch.notch_filter(distance=5 * dc.units.m)`, and dimensionless ones such as `20 %` — now raises `UnitError` with an explanatory message. Previously a dimensionless quantity was silently read as a bare number (`20 %` became 0.2 Hz) and a dimensional one leaked pint's `DimensionalityError`. - `Patch.drop_coords` and `CoordManager.drop_coords` accept a sequence of names as well as bare names, so `patch.drop_coords(["latitude", "longitude"])` works alongside `patch.drop_coords("latitude", "longitude")`. Previously a list or set raised `TypeError` and a tuple or generator was silently ignored; a tuple now drops the named coordinates, and one naming a dimension raises `ParameterError` as a bare dimension name always has. - **Runtime configuration is now two-tier.** `dc.set_config(...)` sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. Temporary, thread/task-local overrides use the new `dc.config_context(...)` context manager, which restores on exit and isolates concurrent overrides via a `ContextVar`. `Spool.map(...)` captures the config active at the call and re-applies it in each worker, so thread- and process-pool workers observe the caller's config regardless of the pool's start method. Unknown config field names now raise instead of being silently ignored. Migrate `with dc.set_config(...)` to `with dc.config_context(...)`. - PRODML 2.1 now supports writing one raw time-by-distance patch with `dc.write` as a standalone HDF5 file. Full PRODML 2.3 conformance requires EPC/XML packaging, which DASCore does not write. diff --git a/tests/test_proc/test_filter.py b/tests/test_proc/test_filter.py index 664fc8e73..a0839b657 100644 --- a/tests/test_proc/test_filter.py +++ b/tests/test_proc/test_filter.py @@ -302,11 +302,21 @@ def test_notch_filter_distance_units(self, random_patch): assert isinstance(filtered_patch, dc.Patch) assert not np.any(np.isnan(filtered_patch.data)) - def test_unitless_coord_with_quantity_raises(self, random_patch): - """A unit-bearing value needs a coordinate with units.""" + @pytest.mark.parametrize( + "value", + (5 * dc.units.m, dc.get_quantity("20%"), dc.get_quantity("0.2")), + ids=("metres", "percent", "dimensionless"), + ) + def test_unitless_coord_with_quantity_raises(self, random_patch, value): + """ + A quantity needs a coordinate with units to be interpreted. + + Dimensionless quantities are rejected too: `20 %` previously + slipped through and was read as 0.2 Hz. + """ patch = random_patch.set_units(distance=None) with pytest.raises(UnitError, match="has no units"): - patch.notch_filter(distance=5 * dc.units.m, q=30) + patch.notch_filter(distance=value, q=30) class TestSavgolFilter: