diff --git a/benchmarks/readme.md b/benchmarks/readme.md index de476b24f..005f77315 100644 --- a/benchmarks/readme.md +++ b/benchmarks/readme.md @@ -17,6 +17,7 @@ pytest benchmarks/ --codspeed pytest benchmarks/test_patch_benchmarks.py --codspeed pytest benchmarks/test_io_benchmarks.py --codspeed pytest benchmarks/test_spool_benchmarks.py --codspeed +pytest benchmarks/test_import_benchmarks.py --codspeed ``` ## Benchmark Structure @@ -27,6 +28,7 @@ Benchmarks are now organized as pytest tests in the `benchmarks/` directory: - `test_io_benchmarks.py` - File I/O operations benchmarks - `test_spool_benchmarks.py` - Spool chunking and selection benchmarks - `test_lookup_benchmarks.py` - In-memory lookups on hot paths (format resolution, remote-cache and IO handle resolution, repeat spool access). These are deliberately small: a change of a few microseconds per lookup is invisible in the end-to-end benchmarks above, because one file read costs far more than the lookups it makes. +- `test_import_benchmarks.py` - Import benchmarks (dascore's own modules; third party dependencies stay warm) Each benchmark uses the `@pytest.mark.benchmark` decorator to automatically measure performance. diff --git a/benchmarks/test_import_benchmarks.py b/benchmarks/test_import_benchmarks.py new file mode 100644 index 000000000..8047af4d3 --- /dev/null +++ b/benchmarks/test_import_benchmarks.py @@ -0,0 +1,53 @@ +"""Benchmarks for importing dascore using pytest-codspeed.""" + +from __future__ import annotations + +import importlib +import sys +import warnings + +import pint +import pytest + + +def _dascore_module_names(): + """Get the names of all currently imported dascore modules.""" + return [x for x in sys.modules if x == "dascore" or x.startswith("dascore.")] + + +class TestImportBenchmarks: + """ + Benchmarks for re-executing dascore's modules. + + Only dascore's own modules are removed from the module cache, so these + measure the cost of executing dascore's module bodies, not the one-time + cost of importing third party dependencies. A fresh interpreter (eg a + CLI call) also pays the latter, but it cannot be measured here; a + subprocess falls outside the region CodSpeed instruments. Instead, see + tests/test_imports.py for the guards which keep slow dependencies out of + the import chain entirely. + """ + + @pytest.fixture() + def restore_dascore_modules(self): + """Put the original dascore modules back after re-importing them.""" + # Import here so third party dependencies are warm before timing, + # even when this file is the only one collected. + importlib.import_module("dascore") + saved = {x: sys.modules[x] for x in _dascore_module_names()} + # Re-importing dascore makes (and installs) a new pint registry. + registry = pint.get_application_registry().get() + # dascore adds a warning filter on import; catch_warnings undoes that. + with warnings.catch_warnings(): + yield + for name in _dascore_module_names(): + del sys.modules[name] + sys.modules.update(saved) + pint.set_application_registry(registry) + + @pytest.mark.benchmark + def test_reimport_dascore(self, restore_dascore_modules): + """Time re-importing the top-level dascore module.""" + for name in _dascore_module_names(): + del sys.modules[name] + importlib.import_module("dascore") diff --git a/benchmarks/test_patch_benchmarks.py b/benchmarks/test_patch_benchmarks.py index 285fa1345..525fb1bbe 100644 --- a/benchmarks/test_patch_benchmarks.py +++ b/benchmarks/test_patch_benchmarks.py @@ -190,6 +190,48 @@ def test_slope_mute(self, example_patch): patch.slope_mute(slopes=(1000, 3000)) +class TestPatchConstructionBenchmarks: + """Benchmarks for the patch construction paths.""" + + @pytest.fixture(scope="module") + def new_data(self, example_patch): + """Data for constructing new patches; built outside the timed call.""" + return np.asarray(example_patch.data) * 2 + + @pytest.fixture(scope="module") + def decimated(self, example_patch): + """A coord manager with one dimension shortened.""" + coord = example_patch.get_coord("time")[::2] + return example_patch.coords.update(time=coord) + + @pytest.mark.benchmark + def test_new_data_only(self, example_patch, new_data): + """Time new when only data changes; coords and attrs are reused.""" + example_patch.new(data=new_data) + + @pytest.mark.benchmark + def test_new_with_coords(self, example_patch, decimated): + """Time new when the coords change, so attrs must be rebuilt.""" + example_patch.new(data=example_patch.data[:, ::2], coords=decimated) + + @pytest.mark.benchmark + def test_new_with_coords_and_attrs(self, example_patch, new_data): + """Time new when both coords and attrs are passed.""" + patch = example_patch + patch.new(data=new_data, coords=patch.coords, attrs=patch.attrs) + + @pytest.mark.benchmark + def test_patch_init(self, example_patch, new_data): + """ + Time the normal constructor. + + This is a control; it should not move, since the strict path is + deliberately left alone. + """ + patch = example_patch + dc.Patch(data=new_data, coords=patch.coords, dims=patch.dims, attrs=patch.attrs) + + class TestTransformBenchmarks: """Benchmarks for patch transform operations.""" @@ -353,6 +395,11 @@ def test_rolling_large_roller_mean(self, big_roller): """Time rolling mean calculation.""" big_roller.mean() + @pytest.mark.benchmark + def test_rolling_mean_full_call(self, example_patch): + """Time a complete rolling mean, including roller construction.""" + example_patch.rolling(time=5, samples=True, center=True).mean() + class TestAlignBenchmarks: """Benchmarks for align_to_coord operation.""" diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 3a5ca423c..57769dfeb 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -163,6 +163,18 @@ def _reduce_time_like(func, data): return np.atleast_1d(out) +def _validate_new_length(length) -> int: + """Ensure a requested coordinate length is a non-negative integer.""" + # bool is an int subclass; True/False are never a sensible length. + if isinstance(length, bool) or not isinstance(length, int | np.integer): + msg = f"change_length requires an integer length, not {length!r}." + raise ParameterError(msg) + if length < 0: + msg = f"change_length requires a non-negative length, not {length}." + raise ParameterError(msg) + return int(length) + + def _get_dtype(value, dtype): """Get the data type based on the first argument.""" if dtype is not None and dtype != "": @@ -399,7 +411,7 @@ def _select_by_sample_array(self, array): msg = "Using an array input for select with samples requires integer dtype." raise CoordError(msg) # Filter out bad indices - if self.ndim > 1: + if self.ndim != 1: msg = "Select only works on 1D coords." raise CoordError(msg) inds = np.arange(len(self)) @@ -1148,7 +1160,12 @@ def change_length(self, length: int) -> Self: Parameters ---------- length - The output length. + The output length. Must be a non-negative integer. + + Raises + ------ + ParameterError + If length is not a non-negative integer. """ msg = f"Coordinate type {self.__class__} does not implement change_length" raise NotImplementedError(msg) @@ -1308,7 +1325,7 @@ def change_length(self, length: int) -> Self: if self.ndim != 1: msg = "change_length only works on 1D coords." raise CoordError(msg) - return get_coord(shape=(length,)) + return get_coord(shape=(_validate_new_length(length),)) def to_summary(self, dims=()) -> CoordSummary: """Get the summary info about the coord.""" @@ -1668,6 +1685,7 @@ def change_length(self, length: int) -> Self: """ # CoordRange is always 1D by construction; keep as an internal invariant. assert self.ndim == 1, "Can only change length for 1D coords." + length = _validate_new_length(length) if len(self) == length: return self # Only the sample count changes; start/step are already valid. diff --git a/dascore/io/dasvader/core.py b/dascore/io/dasvader/core.py index 9f0fb3f66..4c5404e04 100644 --- a/dascore/io/dasvader/core.py +++ b/dascore/io/dasvader/core.py @@ -25,8 +25,9 @@ class DASVaderV1(FiberIO): Notes ----- Legacy DASVader files may contain anonymous JLD2 object references. DASCore - detects those files and raises `DASVaderCompatibilityError` with compatibility - instructions instead of failing inside `h5py`. A known working stack for + reads these references when supported by HDF5 and raises + `DASVaderCompatibilityError` with compatibility instructions when + dereferencing fails. A known working stack for such legacy files is `h5py<3.16` with `HDF5 1.14.x`. """ diff --git a/dascore/io/dasvader/utils.py b/dascore/io/dasvader/utils.py index 7809fb8a3..20800a245 100644 --- a/dascore/io/dasvader/utils.py +++ b/dascore/io/dasvader/utils.py @@ -4,7 +4,6 @@ import h5py import numpy as np -from h5py import h5r from h5py.h5r import Reference, dereference import dascore as dc @@ -77,15 +76,18 @@ def _raise_legacy_ref_error(h5, field_name: str) -> None: def _dereference(h5, value, field_name: str): - """Resolve an HDF5 reference, rejecting legacy anonymous DASVader refs.""" + """Resolve an HDF5 reference or raise a clear compatibility error.""" if not isinstance(value, Reference): return value - if h5r.get_name(value, h5.id) is None: - _raise_legacy_ref_error(h5, field_name) try: return h5[value] except KeyError: - return h5py.Dataset(dereference(value, h5.id)) + # The high-level lookup fails for some references HDF5 can still + # resolve directly, so try that before giving up. + try: + return h5py.Dataset(dereference(value, h5.id)) + except Exception: + _raise_legacy_ref_error(h5, field_name) # --- Metadata parsing diff --git a/dascore/proc/basic.py b/dascore/proc/basic.py index 7c89ad88f..11778b98a 100644 --- a/dascore/proc/basic.py +++ b/dascore/proc/basic.py @@ -318,6 +318,10 @@ def normalize( """ Normalize a patch along a specified dimension. + NaN values are ignored when computing the norm. They remain NaN in the + output but do not affect any other sample. Slices with a norm of zero, + meaning they contain nothing but zeros and NaN, are returned unscaled. + Parameters ---------- dim @@ -348,26 +352,24 @@ def normalize( data = self.data if norm in {"l1", "l2"}: order = int(norm[-1]) - norm_values = np.linalg.norm(self.data, axis=axis, ord=order) + # Equivalent to np.linalg.norm, but skips NaN rather than letting a + # single null blank every sample sharing its slice. The float exponent + # promotes ints so the powers cannot overflow a narrow dtype. + norm_values = np.nansum(np.abs(data) ** float(order), axis=axis) ** (1 / order) + divisor = np.expand_dims(norm_values, axis=axis) elif norm == "max": - norm_values = np.max(np.abs(data), axis=axis) + divisor = np.expand_dims(np.nanmax(np.abs(data), axis=axis), axis=axis) elif norm == "bit": - pass + divisor = np.abs(data) else: msg = ( f"Norm value of {norm} is not supported. " f"Supported values are {('l1', 'l2', 'max', 'bit')}" ) raise ValueError(msg) - if norm == "bit": - new_data = np.divide( - data, np.abs(data), out=np.zeros_like(data), where=np.abs(data) != 0 - ) - else: - expanded_norm = np.expand_dims(norm_values, axis=axis) - new_data = np.divide( - data, expanded_norm, out=np.zeros_like(data), where=expanded_norm != 0 - ) + # A zero divisor means there is nothing but zeros and nulls to scale, so + # divide those by one; the zeros stay zero and the nulls stay null. + new_data = data / np.where(divisor == 0, 1, divisor) return self.new(data=new_data) @@ -385,6 +387,9 @@ def standardize( where u is the mean of the training samples or zero if with_mean=False, and s is the standard deviation of the training samples or one if with_std=False. + NaN values are ignored when computing the mean and standard deviation. They + remain NaN in the output but do not affect any other sample. + Parameters ---------- dim @@ -406,8 +411,8 @@ def standardize( """ axis = self.get_axis(dim) data = self.data - mean = np.mean(data, axis=axis, keepdims=True) - std = np.std(data, axis=axis, keepdims=True) + mean = np.nanmean(data, axis=axis, keepdims=True) + std = np.nanstd(data, axis=axis, keepdims=True) new_data = (data - mean) / std return self.new(data=new_data) @@ -810,6 +815,10 @@ def demedian(patch, dim: str = "time"): """ Remove the median along a given dimension of a DASCore patch. + NaN values are ignored when computing the median, consistent with + [Patch.median](`dascore.proc.aggregate.median`). They remain NaN in the + output but do not affect any other sample. + Parameters ---------- patch : @@ -856,7 +865,7 @@ def demedian(patch, dim: str = "time"): data = patch.data # Compute median along axis, keep dims for broadcasting - med = np.median(data, axis=axis, keepdims=True) + med = np.nanmedian(data, axis=axis, keepdims=True) new_data = data - med @@ -869,6 +878,10 @@ def demean(patch, dim: str = "time"): """ Remove the mean along a given dimension of a DASCore patch. + NaN values are ignored when computing the mean, consistent with + [Patch.mean](`dascore.proc.aggregate.mean`). They remain NaN in the output + but do not affect any other sample. + Parameters ---------- patch : @@ -915,7 +928,7 @@ def demean(patch, dim: str = "time"): data = patch.data # Compute mean along axis, keep dims for broadcasting - mea = np.mean(data, axis=axis, keepdims=True) + mea = np.nanmean(data, axis=axis, keepdims=True) new_data = data - mea diff --git a/dascore/proc/rolling.py b/dascore/proc/rolling.py index 9ded6404c..7d5271222 100644 --- a/dascore/proc/rolling.py +++ b/dascore/proc/rolling.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import dataclass from typing import Any, Literal import numpy as np @@ -11,7 +12,6 @@ from dascore.constants import samples_arg_description from dascore.exceptions import ParameterError from dascore.utils.docs import compose_docstring -from dascore.utils.models import DascoreBaseModel from dascore.utils.patch import ( _maybe_add_history_str, get_dim_axis_value, @@ -41,11 +41,14 @@ """ -class _PatchRollerInfo(DascoreBaseModel): +@dataclass(frozen=True, slots=True) +class _PatchRollerInfo: """ A dataclass for storing info on rolling operation. - Should be subclassed to implement rolling methods. + Should be subclassed to implement rolling methods. This is an ephemeral + internal object created on every rolling call, so it is a plain dataclass + rather than a validated model. """ patch: Any # cant set to patch due to circular import @@ -63,9 +66,10 @@ def get_coords(self): Accounts for centered or non-centered coordinates. If the window length is even, the first half value is used. """ - coord = self.patch.get_coord(self.dim) - if self.step > 1: - coord = coord[:: self.step] + # Without a step the dimension is unchanged; reuse the coord manager. + if self.step == 1: + return self.patch.coords + coord = self.patch.get_coord(self.dim)[:: self.step] return self.patch.coords.update(**{self.dim: coord}) def _get_attrs_with_apply_history(self, func_or_str): @@ -77,6 +81,12 @@ def _get_attrs_with_apply_history(self, func_or_str): hist_str = f"{self.roll_hist}.{func_or_str}()" return _maybe_add_history_str(self.patch.attrs, hist_str) + def _new_patch(self, data, func_or_str): + """Create the output patch from rolled data.""" + coords = self.get_coords() + attrs = self._get_attrs_with_apply_history(func_or_str) + return self.patch.update(data=data, coords=coords, attrs=attrs) + class _NumpyPatchRoller(_PatchRollerInfo): """A class to apply roller operations to patches.""" @@ -92,17 +102,26 @@ def get_start_index(self): return int(out) def _pad_roll_array(self, data): - """Pad.""" + """ + Pad the reduced array with NaNs and align it to the output coordinate. + + The NaNs go at the start of the axis, except when centering, which + moves `num_nans // 2` of them to the end. This is done with a single + allocation rather than a pad followed by a roll. + """ num_nans = 1 + (self.window - 2) // self.step - pad_width = [(0, 0)] * len(data.shape) - pad_width[self.axis] = (num_nans, 0) - padded = np.pad(data, pad_width, constant_values=np.nan) + if not num_nans: # window of one sample; nothing to pad. + return data + shape = list(data.shape) + shape[self.axis] += num_nans + out = np.full(shape, np.nan, dtype=data.dtype) + start = num_nans - num_nans // 2 if self.center else num_nans + slicer = [slice(None, None)] * len(shape) + slicer[self.axis] = slice(start, start + data.shape[self.axis]) + out[tuple(slicer)] = data if self.step == 1: - assert padded.shape == self.patch.data.shape - if self.center: - # roll array along axis to center - padded = np.roll(padded, -(num_nans // 2), axis=self.axis) - return padded + assert out.shape == self.patch.data.shape + return out @compose_docstring(apply_description=rolling_apply_description) def apply(self, function, *args, **kwargs): @@ -128,11 +147,9 @@ def apply(self, function, *args, **kwargs): step_slice[self.axis] = slice(start, None, self.step) # apply function, then pad with NaNs and roll trimmed_slide_view = slide_view[tuple(step_slice)] - raw = function(trimmed_slide_view, *args, axis=-1, **kwargs).astype(np.float64) - out = self._pad_roll_array(raw) - new_coords = self.get_coords() - attrs = self._get_attrs_with_apply_history(function) - return self.patch.update(data=out, coords=new_coords, attrs=attrs) + raw = function(trimmed_slide_view, *args, axis=-1, **kwargs) + out = self._pad_roll_array(np.asarray(raw, dtype=np.float64)) + return self._new_patch(out, function) def mean(self): """Apply mean to moving window.""" @@ -182,21 +199,19 @@ def _get_rolling(self): ) return roll - def _repack_patch(self, df, attrs=None): + def _repack_patch(self, df, func_or_str): """Repack patch into dataframe.""" data = df.values if not self.axis else df.T.values # get rid of extra dims if original data doesn't have them. if len(data.shape) != len(self.patch.data.shape): data = np.squeeze(data) - coords = self.get_coords() - return self.patch.update(data=data, coords=coords, attrs=attrs) + return self._new_patch(data, func_or_str) def _call_rolling_func(self, name, *args, **kwargs): """Helper function for calling a rolling function.""" rolling = self._get_rolling() df = getattr(rolling, name)(*args, **kwargs) - attrs = self._get_attrs_with_apply_history(name) - return self._repack_patch(df, attrs=attrs) + return self._repack_patch(df, name) @compose_docstring(apply_description=rolling_apply_description) def apply(self, function, *args, **kwargs): @@ -204,8 +219,7 @@ def apply(self, function, *args, **kwargs): {apply_description} """ df = self._get_rolling().apply(function, args=args, kwargs=kwargs) - attrs = self._get_attrs_with_apply_history(function) - return self._repack_patch(df, attrs=attrs) + return self._repack_patch(df, function) def mean(self): """Apply mean.""" diff --git a/dascore/transform/fbe.py b/dascore/transform/fbe.py index 2b9bd9598..20d7f16a9 100644 --- a/dascore/transform/fbe.py +++ b/dascore/transform/fbe.py @@ -40,8 +40,8 @@ def fbe( can be used for downsampling of the resulting patch. See also [rolling](`dascore.Patch.rolling`) db - Return patch data in decibel [dB] instead of orginal units. - Decibel is calculated as 20 * log10( sqrt(mean(x^2))) ). + Return patch data in decibel [dB] instead of original units. + Decibel is calculated as 20 * log10( sqrt(mean(x^2)) ). **kwargs Used to specify the dimension and asociated frequency, wavelength, or equivalent limits. For example time=(1, 100) applies a time-dimension bandpass diff --git a/dascore/utils/attrs.py b/dascore/utils/attrs.py index 50d9ab78b..2f86d3885 100644 --- a/dascore/utils/attrs.py +++ b/dascore/utils/attrs.py @@ -13,13 +13,23 @@ import dascore as dc from dascore.constants import attr_conflict_description -from dascore.exceptions import AttributeMergeError +from dascore.exceptions import AttributeMergeError, ParameterError from dascore.utils.docs import compose_docstring from dascore.utils.misc import ( _dict_list_diffs, iterate, ) +_VALID_CONFLICT_VALUES = ("drop", "raise", "keep_first") + + +def validate_conflict(conflict: str) -> Literal["drop", "raise", "keep_first"]: + """Ensure a conflict(s) argument is a supported value.""" + if conflict not in _VALID_CONFLICT_VALUES: + msg = f"conflict must be one of {_VALID_CONFLICT_VALUES}, got {conflict!r}." + raise ParameterError(msg) + return conflict + @compose_docstring(conflict_desc=attr_conflict_description) def combine_patch_attrs( @@ -39,6 +49,7 @@ def combine_patch_attrs( drop_attrs If provided, attributes which should be dropped. """ + validate_conflict(conflicts) def _to_patch_attrs(model): """Normalize supported attr-like inputs to PatchAttrs.""" diff --git a/dascore/utils/chunk_plan.py b/dascore/utils/chunk_plan.py index dfc3f27b0..4b2b2919b 100644 --- a/dascore/utils/chunk_plan.py +++ b/dascore/utils/chunk_plan.py @@ -30,6 +30,7 @@ InvalidSpoolQueryError, ParameterError, ) +from dascore.utils.attrs import validate_conflict from dascore.utils.chunk import get_intervals from dascore.utils.misc import get_middle_value, is_range from dascore.utils.pd import _remove_overlaps, get_interval_columns @@ -474,6 +475,9 @@ def build_chunk_plan( f"kwargs: {kwargs}" ) raise ParameterError(msg) + # Fail here rather than later at assembly, so the error points at the + # offending chunk call. See #804. + validate_conflict(conflict) ((name, value),) = kwargs.items() value = None if value is Ellipsis else value merge_mode = pd.isnull(value) @@ -492,9 +496,6 @@ def build_chunk_plan( if missing_dim not in ("raise", "drop"): msg = f"missing_dim must be 'raise' or 'drop', got {missing_dim!r}" raise ParameterError(msg) - if conflict not in ("drop", "raise", "keep_first"): - msg = "conflict must be 'drop', 'raise', or 'keep_first', " f"got {conflict!r}" - raise ParameterError(msg) min_name, max_name = f"{name}_min", f"{name}_max" if min_name not in df.columns and not df.empty: diff --git a/dascore/utils/pd.py b/dascore/utils/pd.py index deab137c2..e05db70af 100644 --- a/dascore/utils/pd.py +++ b/dascore/utils/pd.py @@ -284,9 +284,34 @@ def _filter_equality(query_dict, df, bool_index): return bool_index +def _check_misdirected_range_query(key, val, df): + """ + Raise if a range query was aimed at an interval column. + + Columns like time_min/time_max hold the limits of each row, and a + collection of values applied to one is a membership (isin) check. An open + bound (... or None) in such a collection is meaningless, and signals a + range query which belongs on the dimension instead, eg + time_min=(t1, ...) should be time=(t1, ...). Without this the query would + silently match nothing. + """ + base = key[:-4] if key.endswith(("_min", "_max")) else None + if base is None or not {f"{base}_min", f"{base}_max"}.issubset(set(df.columns)): + return + if not any(x is ... or x is None for x in val): + return + msg = ( + f"An open bound (... or None) is not valid in the query for column " + f"'{key}'; a collection of values is an isin check, not a range. " + f"Use {base}=(min, max) to query a range of {base} values." + ) + raise ParameterError(msg) + + def _filter_contains(query_dict, df, bool_index): """Filter based on rows containing specified values.""" for key, val in query_dict.items(): + _check_misdirected_range_query(key, val, df) bool_index = np.logical_and(bool_index, df[key].isin(val)) return bool_index @@ -320,6 +345,16 @@ def _filter_multicolumn_range(query_dict, df, bool_index): return bool_index +def _convert_range_bounds(range_tuple, func): + """ + Apply a time conversion to each bound of a range. + + Unbounded (None) ends are left alone; converting them would produce NaT, + which compares False against everything and would silently empty the query. + """ + return tuple(None if x is None else func(x) for x in range_tuple) + + def _convert_times(df, some_dict): """Convert query values to datetime/timedelta values.""" if not some_dict: @@ -331,12 +366,12 @@ def _convert_times(df, some_dict): non_min_max_cols & set(some_dict) ) for key in datetime_keys: - some_dict[key] = to_datetime64(some_dict[key]) + some_dict[key] = _convert_range_bounds(some_dict[key], to_datetime64) # convert queries related to time delta into timedelta64 timedelta_cols = set(df.select_dtypes(include=np.timedelta64).columns) timedelta_keys = timedelta_cols & set(some_dict) for key in timedelta_keys: - some_dict[key] = to_timedelta64(some_dict[key]) + some_dict[key] = _convert_range_bounds(some_dict[key], to_timedelta64) return some_dict diff --git a/docs/recipes/smoothing.qmd b/docs/recipes/smoothing.qmd index c5e507a96..ac62bcdf7 100644 --- a/docs/recipes/smoothing.qmd +++ b/docs/recipes/smoothing.qmd @@ -1,7 +1,7 @@ --- title: "Patch Smoothing" execute: - warn: false + warning: false --- This recipe compares several smoothing strategies. diff --git a/docs/tutorial/patch.qmd b/docs/tutorial/patch.qmd index 3ea33cdb2..126ed716f 100644 --- a/docs/tutorial/patch.qmd +++ b/docs/tutorial/patch.qmd @@ -876,9 +876,9 @@ patch_reduced = patch.add.reduce("distance") ``` -```{.note} -PatchUFunc.reduce/accumulate accept dim (mapped internally to axis), while numpy ufuncs accept axis. -``` +:::{.callout-note} +`PatchUFunc.reduce`/`accumulate` accept `dim` (mapped internally to `axis`), while numpy ufuncs accept `axis`. +::: For more advanced usage, you can create patch ufuncs. diff --git a/docs/tutorial/processing.qmd b/docs/tutorial/processing.qmd index 62cd3c53c..34e728655 100644 --- a/docs/tutorial/processing.qmd +++ b/docs/tutorial/processing.qmd @@ -161,7 +161,7 @@ Here is an example of using a rolling mean to smooth along the time axis: import dascore as dc patch = dc.get_example_patch("example_event_1") -# Apply moving mean window over every 10 samples +# Apply moving mean window over every 50 samples dt = patch.get_coord('time').step smoothed = patch.rolling(time=50*dt).mean() diff --git a/docs/tutorial/spool.qmd b/docs/tutorial/spool.qmd index 332e41362..54008b5e4 100644 --- a/docs/tutorial/spool.qmd +++ b/docs/tutorial/spool.qmd @@ -187,10 +187,10 @@ import numpy as np spool = dc.get_example_spool() # Get an array of integers which indicate the index of included patches -bool_array = np.array([2, 0]) +index_array = np.array([2, 0]) # create a new spool with patch 2 and patch 0. -new = spool[bool_array] +new = spool[index_array] ``` # get_contents @@ -282,7 +282,7 @@ print(merged[0].coords) The [`map`](`dascore.core.spool.BaseSpool.map`) method applies a function to all patches in the spool. It provides an efficient way to process large datasets, especially when combined with clients (aka executors). -For example, calculating the maximum value for each channel (distance) for 4 second increments with 1 second overlap can be done like so: +For example, calculating the maximum value for each channel (distance) for 5 second increments with 1 second overlap can be done like so: ```{python} import dascore as dc diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index 8c4fd21d9..857142ec9 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -2248,6 +2248,38 @@ def test_not_implemented_in_baseclass(self, evenly_sampled_coord): with pytest.raises(NotImplementedError): BaseCoord.change_length(coord, 10) + @pytest.mark.parametrize("length", [-1, -10]) + def test_negative_length_raises( + self, evenly_sampled_coord, basic_non_coord, length + ): + """Ensure a negative length is rejected rather than making a bad coord.""" + for coord in (evenly_sampled_coord, basic_non_coord): + with pytest.raises(ParameterError, match="non-negative"): + coord.change_length(length) + + @pytest.mark.parametrize("length", [2.5, 3.0, "3", None, True, False]) + def test_non_integer_length_raises( + self, evenly_sampled_coord, basic_non_coord, length + ): + """Ensure non integer lengths are rejected.""" + for coord in (evenly_sampled_coord, basic_non_coord): + with pytest.raises(ParameterError, match="integer length"): + coord.change_length(length) + + def test_zero_length(self, evenly_sampled_coord, basic_non_coord): + """Ensure a length of zero produces an empty coord.""" + for coord in (evenly_sampled_coord, basic_non_coord): + assert len(coord.change_length(0)) == 0 + + def test_zero_length_keeps_metadata(self, evenly_sampled_float_coord_with_units): + """Ensure an emptied coord keeps its units, step and dtype.""" + coord = evenly_sampled_float_coord_with_units + out = coord.change_length(0) + assert len(out) == 0 + assert out.units == coord.units + assert out.step == coord.step + assert out.dtype == coord.dtype + class TestIssues: """Tests for special issues related to coords.""" @@ -2652,6 +2684,13 @@ def test_select_sample_array_2d_raises(self, coord_2d): with pytest.raises(CoordError, match="1D coords"): coord_2d.select(np.array([0, 1]), samples=True) + def test_select_sample_array_0d_raises(self): + """A rank-0 coord must also be rejected, not just >1D.""" + coord_0d = CoordPartial(shape=()) + assert coord_0d.ndim == 0 + with pytest.raises(CoordError, match="1D coords"): + coord_0d.select(np.array([0, 1]), samples=True) + def test_get_sample_count_2d_raises(self, coord_2d): """get_sample_count requires a 1D coord.""" with pytest.raises(CoordError, match="1D coords"): diff --git a/tests/test_core/test_patch.py b/tests/test_core/test_patch.py index 9c3096d02..dcc5befbc 100644 --- a/tests/test_core/test_patch.py +++ b/tests/test_core/test_patch.py @@ -18,7 +18,12 @@ from dascore.core import Patch from dascore.core.coords import BaseCoord, CoordRange from dascore.core.summary import PatchSummary -from dascore.exceptions import CoordError, ParameterError, PatchAttributeError +from dascore.exceptions import ( + CoordDataError, + CoordError, + ParameterError, + PatchAttributeError, +) from dascore.io.core import ( _scan_result_to_summary, _select_patch_from_spool, @@ -99,31 +104,6 @@ def patch_complex_coords(self): out = dict(data=array, coords=coords, attrs=attrs, dims=dims) return Patch(**out) - @pytest.fixture(scope="class") - def test_conflicting_attrs_coords_raises(self): - """Patch for testing conflicting coordinates/attributes.""" - array = random_state.random((10, 10)) - # create attrs with coordinate metadata; these should now be rejected. - attrs = dict( - distance_step=10, - time_step=dc.to_timedelta64(1), - distance_min=1000, - distance_max=2002, - time_min=dc.to_datetime64("2017-01-01"), - time_max=dc.to_datetime64("2019-01-01"), - ) - # create coords - coords = dict( - time=dc.to_datetime64(np.cumsum(random_state.random(10))), - distance=random_state.random(10), - ) - # assemble and output. - dims = ("distance", "time") - out = dict(data=array, coords=coords, attrs=attrs, dims=dims) - msg = "coordinate metadata" - with pytest.raises(ValueError, match=msg): - dc.Patch(**out) - def test_start_time_inferred_from_dt64_coords(self, random_dt_coord): """Ensure the time_min and time_max attrs can be inferred from coord time.""" patch = random_dt_coord @@ -388,6 +368,36 @@ def test_new_dims_renames_dims(self, random_patch): out = random_patch.new(dims=dims) assert out.dims == dims + def test_new_data_only_validates_shape(self, random_patch): + """New should still reject data which doesn't match the coords.""" + data = random_patch.data[:-1] + with pytest.raises(CoordDataError): + random_patch.new(data=data) + + def test_new_coords_validates_shape(self, random_patch): + """New should reject data which doesn't match passed coords.""" + coords = random_patch.coords + with pytest.raises(CoordDataError): + random_patch.new(data=random_patch.data[:, :-1], coords=coords) + + def test_new_same_coords_matches_default(self, random_patch): + """Passing the patch's own coords should match passing nothing.""" + data = random_patch.data * 2 + out1 = random_patch.new(data=data) + out2 = random_patch.new(data=data, coords=random_patch.coords) + assert out1.equals(out2, only_required_attrs=False) + + def test_new_no_args_equals_input(self, random_patch): + """New with no arguments should reproduce the patch.""" + assert random_patch.new().equals(random_patch, only_required_attrs=False) + + def test_new_from_patch_takes_over_metadata(self, random_patch): + """Passing a patch as data should adopt its coords and attrs.""" + other = random_patch.decimate(time=2) + out = random_patch.new(data=other) + assert out.coords == other.coords + assert out.shape == other.shape + class TestPatchSummary: """Tests for patch summary helpers and selection fallbacks.""" diff --git a/tests/test_core/test_patch_chunk.py b/tests/test_core/test_patch_chunk.py index 278e40f04..ec4cef3c4 100644 --- a/tests/test_core/test_patch_chunk.py +++ b/tests/test_core/test_patch_chunk.py @@ -540,6 +540,16 @@ def test_attrs_conflict(self, adjacent_spool_different_attrs): patch = out[0] assert isinstance(patch, dc.Patch) + def test_invalid_conflict_raises(self, adjacent_spool_different_attrs): + """ + An unrecognized conflict value should raise rather than silently + selecting undocumented behavior. See #804. + """ + spool = adjacent_spool_different_attrs + for bad_value in ("banana", "", None): + with pytest.raises(ParameterError, match="conflict must be one of"): + spool.chunk(time=..., conflict=bad_value) + def test_chunk_patches_with_non_coord(self, random_patch): """Tests for chunking when some patches have non coordinate dimensions.""" patches = [random_patch.mean("time") for _ in range(3)] diff --git a/tests/test_io/test_dasvader/test_dasvader.py b/tests/test_io/test_dasvader/test_dasvader.py index 280b7152e..6c57797c5 100644 --- a/tests/test_io/test_dasvader/test_dasvader.py +++ b/tests/test_io/test_dasvader/test_dasvader.py @@ -4,6 +4,7 @@ from __future__ import annotations +import warnings from dataclasses import dataclass from pathlib import Path @@ -13,7 +14,11 @@ from h5py.h5r import Reference import dascore as dc -from dascore.exceptions import DependencyError, UnknownFiberFormatError +from dascore.exceptions import ( + DASVaderCompatibilityError, + DependencyError, + UnknownFiberFormatError, +) from dascore.io.dasvader.utils import _dereference, _julia_ms_to_datetime64 from dascore.utils.downloader import fetch @@ -207,10 +212,20 @@ def test_legacy_file_raises_clear_error(self, legacy_das_vader_path): assert patch.attrs is not None def test_legacy_file_scan_warns_and_skips(self, legacy_das_vader_path): - """Scan should surface compatibility guidance as a warning, not an error.""" - with pytest.warns(UserWarning, match="legacy DASVader JLD2 file"): + """ + Scan should surface compatibility guidance as a warning, not an error. + + On an HDF5 stack which can resolve the file's anonymous references + it simply scans, so only the unsupported case asserts the warning. + """ + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") out = dc.scan(legacy_das_vader_path) - assert out == [] + messages = [str(x.message) for x in caught] + if out == []: + assert any("legacy DASVader JLD2 file" in x for x in messages) + else: + assert len(out) == 1 def test_non_dasvader_jld2_is_not_claimed(self, tmp_path): """Non-DASVader JLD2/HDF5 files should not be identified as DASVader.""" @@ -280,3 +295,32 @@ def test_dereference_returns_non_reference(self): """Non-reference values should be returned unchanged.""" value = np.float64(5_000.0) assert _dereference(None, value, "PulseRateFreq") == value + + def test_dereference_anonymous_reference(self, tmp_path): + """Anonymous references should be read when supported by HDF5.""" + path = tmp_path / "anonymous_reference.h5" + with h5py.File(path, "w") as resource: + target = resource.create_dataset("target", data=np.array([1])) + resource.create_dataset("reference", data=target.ref, dtype=h5py.ref_dtype) + del resource["target"] + reference = resource["reference"][()] + + assert h5py.h5r.get_name(reference, resource.id) is None + resolved = _dereference(resource, reference, "htime") + + assert resolved[0] == 1 + + def test_dereference_failure(self): + """Failed references should raise a clear compatibility error.""" + + class BrokenResource: + """Minimal HDF5 resource whose references cannot be resolved.""" + + filename = "legacy.jld2" + + def __getitem__(self, value): + raise KeyError(value) + + match = r"legacy\.jld2.*'htime'.*h5py<3\.16" + with pytest.raises(DASVaderCompatibilityError, match=match): + _dereference(BrokenResource(), Reference(), "htime") diff --git a/tests/test_proc/test_basic.py b/tests/test_proc/test_basic.py index 0bada7f78..ce1715d93 100644 --- a/tests/test_proc/test_basic.py +++ b/tests/test_proc/test_basic.py @@ -18,6 +18,13 @@ TEST_OPS = tuple(getattr(operator, x) for x in OP_NAMES) +def _patch_with_nan(patch, index=(0, 0)): + """Return a float patch with a single NaN so slice contamination shows.""" + data = np.asarray(patch.data, dtype=np.float64).copy() + data[index] = np.nan + return patch.new(data=data) + + @pytest.fixture(scope="session") def random_complex_patch(random_patch): """Swap out data for complex data.""" @@ -198,6 +205,56 @@ def test_zero_channels(self, random_patch): assert np.all(norm.data[0, :] == 0.0) assert np.all(norm.data[:, 0] == 0.0) + @pytest.mark.parametrize("norm", ["l1", "l2", "max"]) + @pytest.mark.parametrize("dim", ["time", "distance"]) + def test_nan_does_not_contaminate_slice(self, random_patch, dim, norm): + """A single NaN should not blank every value sharing its slice.""" + patch = _patch_with_nan(random_patch) + out = patch.normalize(dim, norm=norm) + assert np.isnan(out.data).sum() == 1 + + @pytest.mark.filterwarnings("ignore:All-NaN slice encountered") + @pytest.mark.parametrize("norm", ["l1", "l2", "max"]) + @pytest.mark.parametrize("dim", ["time", "distance"]) + def test_all_nan_slice_stays_null(self, random_patch, dim, norm): + """A completely null slice should stay null rather than become zeros.""" + data = np.asarray(random_patch.data, dtype=np.float64).copy() + # Null the first slice reduced by norm (patch is 2D, so the other axis). + other_axis = 1 - random_patch.get_axis(dim) + null_slice = tuple(0 if i == other_axis else slice(None) for i in range(2)) + data[null_slice] = np.nan + patch = random_patch.new(data=data) + + out = patch.normalize(dim, norm=norm) + + assert np.all(np.isnan(out.data[null_slice])) + assert not np.any(np.isnan(np.delete(out.data, 0, axis=other_axis))) + + @pytest.mark.parametrize("norm", ["l1", "l2", "max", "bit"]) + def test_null_in_zero_slice_stays_null(self, random_patch, norm): + """A null in an otherwise zero slice should stay null, not become zero.""" + data = np.zeros(random_patch.shape, dtype=np.float64) + data[0, 0] = np.nan + patch = random_patch.new(data=data) + + out = patch.normalize("time", norm=norm) + + assert np.isnan(out.data[0, 0]) + assert np.all(out.data[0, 1:] == 0) + + @pytest.mark.parametrize("norm", ["l1", "l2", "max", "bit"]) + def test_int_data(self, random_patch, norm): + """Integer data should normalize to floats without overflowing.""" + # 100 is large enough that squaring it overflows int8. + data = np.full(random_patch.shape, 100, dtype=np.int8) + samples = data.shape[random_patch.get_axis("time")] + expected = {"l1": 1 / samples, "l2": 1 / np.sqrt(samples), "max": 1, "bit": 1} + + out = random_patch.new(data=data).normalize("time", norm=norm) + + assert np.issubdtype(out.data.dtype, np.floating) + assert np.allclose(out.data, expected[norm]) + class TestStandardize: """Tests for standardization.""" @@ -211,6 +268,16 @@ def test_base_case(self, random_patch): out = random_patch.standardize("time") assert not np.any(pd.isnull(out.data)) + @pytest.mark.parametrize("dim", ["time", "distance"]) + def test_nan_does_not_contaminate_slice(self, random_patch, dim): + """A single NaN should not blank every value sharing its slice.""" + patch = _patch_with_nan(random_patch) + out = patch.standardize(dim) + axis = out.get_axis(dim) + assert np.isnan(out.data).sum() == 1 + assert np.allclose(np.nanmean(out.data, axis=axis), 0) + assert np.allclose(np.nanstd(out.data, axis=axis), 1) + def test_std(self, random_patch): """Ensure after operation standard deviations are 1.""" dims = random_patch.dims @@ -861,6 +928,15 @@ def test_demedian(self, random_patch, dim): medians = np.median(dem.data, axis=dem.get_axis(dim)) assert np.allclose(medians, 0) + @pytest.mark.parametrize("dim", ["time", "distance"]) + def test_nan_does_not_contaminate_slice(self, random_patch, dim): + """A single NaN should not blank every value sharing its slice.""" + patch = _patch_with_nan(random_patch) + out = patch.demedian(dim=dim) + assert np.isnan(out.data).sum() == 1 + medians = np.nanmedian(out.data, axis=out.get_axis(dim)) + assert np.allclose(medians, 0) + class TestDemean: """Tests for demean of data.""" @@ -873,3 +949,12 @@ def test_demean(self, random_patch, dim): dem = new.demean(dim=dim) means = np.mean(dem.data, axis=dem.get_axis(dim)) assert np.allclose(means, 0) + + @pytest.mark.parametrize("dim", ["time", "distance"]) + def test_nan_does_not_contaminate_slice(self, random_patch, dim): + """A single NaN should not blank every value sharing its slice.""" + patch = _patch_with_nan(random_patch) + out = patch.demean(dim=dim) + assert np.isnan(out.data).sum() == 1 + means = np.nanmean(out.data, axis=out.get_axis(dim)) + assert np.allclose(means, 0) diff --git a/tests/test_proc/test_rolling.py b/tests/test_proc/test_rolling.py index d33d99dc9..020c7e851 100644 --- a/tests/test_proc/test_rolling.py +++ b/tests/test_proc/test_rolling.py @@ -252,6 +252,78 @@ def percentile_plus(frame, q, offset=0, axis=None): assert all_close(out, expected) +class TestRollingMetadata: + """Ensure rolling output carries over the input patch's metadata.""" + + @pytest.fixture(scope="class") + def unit_patch(self, random_patch_with_lat_lon): + """A patch with units set on its data and all of its coords.""" + patch = random_patch_with_lat_lon + return patch.set_units("strain", distance="m", time="s", latitude="deg") + + @pytest.mark.parametrize("engine", ("numpy", "pandas")) + def test_coords_unchanged_without_step(self, random_patch, engine): + """Without a step the rolling dim is unchanged, so are the coords.""" + out = random_patch.rolling(time=10, samples=True, engine=engine).mean() + assert out.coords == random_patch.coords + + @pytest.mark.parametrize("engine", ("numpy", "pandas")) + def test_units_preserved(self, unit_patch, engine): + """Data units and units of every coord should survive rolling.""" + out = unit_patch.rolling(time=10, samples=True, engine=engine).mean() + assert out.attrs.data_units == unit_patch.attrs.data_units + for name in unit_patch.coords.coord_map: + assert out.get_coord(name).units == unit_patch.get_coord(name).units + + @pytest.mark.parametrize("engine", ("numpy", "pandas")) + def test_attrs_preserved(self, random_patch, engine): + """Non-coordinate attrs should be unaffected by rolling.""" + patch = random_patch.update_attrs(tag="bob", station="wat") + out = patch.rolling(time=10, samples=True, engine=engine).mean() + assert out.attrs.tag == "bob" + assert out.attrs.station == "wat" + + # The numpy engine routes its reductions through apply, pandas does not. + @pytest.mark.parametrize( + "engine,suffix", (("numpy", ".apply(mean)"), ("pandas", ".mean()")) + ) + def test_history_appended(self, random_patch, engine, suffix): + """A single history entry naming the operation should be added.""" + out = random_patch.rolling(time=10, samples=True, engine=engine).mean() + history = list(out.attrs.history) + assert len(history) == len(random_patch.attrs.history) + 1 + expected = ( + f"rolling(time=10, step=1, overlap=None, " + f"center=False, engine={engine}){suffix}" + ) + assert history[-1] == expected + + @pytest.mark.parametrize("engine", ("numpy", "pandas")) + def test_non_dim_coords_preserved(self, random_patch_with_lat_lon, engine): + """Coords which don't change shape should be kept as they were.""" + patch = random_patch_with_lat_lon + out = patch.rolling(time=10, samples=True, engine=engine).mean() + assert set(out.coords.coord_map) == set(patch.coords.coord_map) + for name in set(patch.coords.coord_map) - {"time"}: + assert out.get_coord(name) == patch.get_coord(name) + assert np.array_equal(out.get_array(name), patch.get_array(name)) + + @pytest.mark.parametrize("engine", ("numpy", "pandas")) + @pytest.mark.parametrize("step", (1, 3)) + def test_history_is_tuple(self, random_patch, engine, step): + """ + History must stay a tuple. + + The step==1 path uses model_copy, which does no type coercion, so a + list would silently produce attrs unequal to the validated form. + """ + out = random_patch.rolling( + time=10, samples=True, step=step, engine=engine + ).mean() + assert isinstance(out.attrs.history, tuple) + assert out.attrs == out.attrs.update(history=out.attrs.history) + + class TestNumpyVsPandasRolling: """Ensure numpy rolling return the same results as pandas rolling.""" diff --git a/tests/test_utils/test_attrs_utils.py b/tests/test_utils/test_attrs_utils.py index 2ba5e3cb5..36417e1b3 100644 --- a/tests/test_utils/test_attrs_utils.py +++ b/tests/test_utils/test_attrs_utils.py @@ -7,6 +7,7 @@ import pytest from dascore import PatchAttrs +from dascore.exceptions import ParameterError from dascore.utils.attrs import combine_patch_attrs @@ -27,6 +28,12 @@ def test_drop(self): out = combine_patch_attrs([pa1, pa2], drop_attrs="history") assert isinstance(out, PatchAttrs) + def test_invalid_conflicts_raises(self): + """An unsupported conflicts value should raise. See #804.""" + pa1, pa2 = PatchAttrs(tag="bob"), PatchAttrs(tag="bill") + with pytest.raises(ParameterError, match="conflict must be one of"): + combine_patch_attrs([pa1, pa2], conflicts="banana") + def test_conflicts(self): """Ensure when non-dim fields aren't equal merge raises.""" pa1 = PatchAttrs(tag="bob", another=2, same=42) diff --git a/tests/test_utils/test_chunk.py b/tests/test_utils/test_chunk.py index 3f12ada00..412f5c003 100644 --- a/tests/test_utils/test_chunk.py +++ b/tests/test_utils/test_chunk.py @@ -9,7 +9,7 @@ import pytest import dascore as dc -from dascore.exceptions import ChunkError +from dascore.exceptions import ChunkError, ParameterError from dascore.utils.chunk import get_intervals from dascore.utils.chunk_plan import build_chunk_plan from dascore.utils.time import to_timedelta64 @@ -231,6 +231,11 @@ def test_unknown_dim_raises(self, contiguous_df): with pytest.raises(ChunkError, match="Time"): build_chunk_plan(contiguous_df, Time=10) + def test_invalid_conflict_raises(self, contiguous_df): + """An unsupported conflict value raises at the chunk call. See #804.""" + with pytest.raises(ParameterError, match="conflict must be one of"): + build_chunk_plan(contiguous_df, time=None, conflict="banana") + class TestChunkPlanToMerge: """Merge-mode planning on raw dataframes.""" diff --git a/tests/test_utils/test_pd.py b/tests/test_utils/test_pd.py index 894719e32..fec9b48a2 100644 --- a/tests/test_utils/test_pd.py +++ b/tests/test_utils/test_pd.py @@ -249,6 +249,78 @@ def test_timedelta_columns(self, example_df_timedeltas): out = filter_df(df, time_step_min=0.5, time_step_max=2) assert out.all() + def test_open_ended_datetime_range(self, example_df_2): + """ + An unbounded end of a range on a datetime column should not exclude + everything. See #808. + """ + # 'time' must be a plain column, ie not paired with time_min/time_max. + df = pd.DataFrame({"time": example_df_2["time_min"]}) + cutoff = df["time"].iloc[2] + assert np.all(filter_df(df, time_min=cutoff) == (df["time"] >= cutoff)) + assert np.all(filter_df(df, time_max=cutoff) == (df["time"] <= cutoff)) + + def test_open_ended_timedelta_range(self, example_df_2): + """Same as above, but for timedelta columns.""" + df = example_df_2.assign(step=example_df_2["time_step"] * range(5)) + cutoff = df["step"].iloc[2] + assert np.all(filter_df(df, step_min=cutoff) == (df["step"] >= cutoff)) + assert np.all(filter_df(df, step_max=cutoff) == (df["step"] <= cutoff)) + + @pytest.mark.parametrize( + "val", + [ + ("2020-01-03", ...), + (..., "2020-01-03"), + ("2020-01-03", None), + (None, "2020-01-03"), + ], + ) + def test_open_bound_on_interval_column_raises(self, example_df_2, val): + """ + An open bound in a membership query on an interval column is a range + query aimed at the wrong key; it should raise rather than silently + return an empty result. See #808. + """ + with pytest.raises(ParameterError, match=r"Use time=\(min, max\)"): + filter_df(example_df_2, time_min=val) + + def test_closed_range_on_interval_column_still_isin(self, example_df_2): + """ + Without an open bound the query is ambiguous, so the documented isin + behavior is kept. + """ + vals = list(example_df_2["bp_min"].iloc[:2]) + out = filter_df(example_df_2, bp_min=tuple(vals)) + assert np.all(out == example_df_2["bp_min"].isin(vals)) + + def test_ellipsis_kept_for_non_interval_column(self, example_df_2): + """ + Columns with no min/max pair are plain isin checks; an ellipsis there + contributes nothing but must not break the rest of the collection. + """ + out = filter_df(example_df_2, first_name=("Jason", ...)) + assert np.all(out == example_df_2["first_name"].isin(["Jason"])) + + def test_open_bound_ignored_for_unknown_column(self, example_df_2): + """ + Unknown columns are forwarded to patch level select, so an open bound + in one of them must not raise here. + """ + out = filter_df(example_df_2, not_a_column=(1, ...), ignore_bad_kwargs=True) + assert out.all() + + def test_spool_select_open_bound_on_interval_column(self, random_spool): + """ + The user facing path which prompted this: spool.select(time_min=...). + + The spool rejects the unknown name before the interval-column check + below it can run, but the point stands: it raises rather than + silently returning nothing. + """ + with pytest.raises(ParameterError, match="neither an attribute nor a coord"): + random_spool.select(time_min=("2020-01-01", ...)) + class TestAdjustSegments: """Tests for adjusting segments of dataframes."""