diff --git a/dascore/proc/filter.py b/dascore/proc/filter.py index 85b3878cf..42c9fa591 100644 --- a/dascore/proc/filter.py +++ b/dascore/proc/filter.py @@ -319,7 +319,17 @@ 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: + # 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 quantity cannot be interpreted " + "against it. 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..d07ae05ce 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") @@ -389,10 +391,39 @@ 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) +@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..9ce98e446 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 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 35850002b..a0839b657 100644 --- a/tests/test_proc/test_filter.py +++ b/tests/test_proc/test_filter.py @@ -302,6 +302,22 @@ def test_notch_filter_distance_units(self, random_patch): assert isinstance(filtered_patch, dc.Patch) assert not np.any(np.isnan(filtered_patch.data)) + @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=value, 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."""