From f56a25180cec113d56152845c4e9303bd4d73084 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Wed, 5 Aug 2026 09:14:37 +0200 Subject: [PATCH 1/8] Type the units module's dynamic attribute access from dascore.units import m gave Unknown, which silently absorbed every check downstream. A name reaching __getattr__ is always a non-empty identifier, so it resolves to a quantity or raises. --- dascore/units.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dascore/units.py b/dascore/units.py index 21ab5dfcc..c5479bd6b 100644 --- a/dascore/units.py +++ b/dascore/units.py @@ -7,7 +7,7 @@ from functools import cache from threading import RLock from types import EllipsisType -from typing import Any, TypeVar +from typing import Any, TypeVar, cast import numpy as np import pandas as pd @@ -520,7 +520,7 @@ def maybe_convert_percent_to_fraction(obj): return out -def __getattr__(name): +def __getattr__(name: str) -> Quantity: """ Allows arbitrary units (quantities) to be imported from this module. @@ -530,5 +530,9 @@ def __getattr__(name): is the same as from dascore.units import get_quantity m = get_quantity("m") + + A name is always a non-empty identifier here, so this either resolves + to a quantity or raises UndefinedUnitError; it never returns None the + way get_quantity does for None or an empty string. """ - return get_quantity(name) + return cast("Quantity", get_quantity(name)) From 4116feb77cefc543f6409eb9a592fc1c1a5cc4c5 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:07:41 +0200 Subject: [PATCH 2/8] Correct the coord-mapping and scan-payload input types The coord map type was wrong in three separate ways, all of which ty flagged as callers failing to match it: - A tuple key was allowed, but get_coord_manager rejects any key that is not a dimension name, so no caller could ever use one. - Only ndarray was allowed as data, but a list or range works and both the dispersion and taup transforms pass one. - dict was used rather than Mapping, so a caller holding a narrower value type did not match even when every value was valid. Patch.new declared its own version of this type; it now shares the alias. A bare tuple stays reserved for the (dimension, data) form. Patch.drop_coords advertised Collection[str], which never worked: a list or set raises TypeError on the set intersection and a tuple silently drops nothing. It takes plain strings, as every caller already passes. _scan_payload_to_summary defaulted source_format and source_version to None, which PatchSummary's before-validator maps to "". Default them to the string the model stores so the annotation matches the field. --- dascore/core/coordmanager.py | 16 ++++++++++++---- dascore/io/core.py | 6 ++++-- dascore/proc/basic.py | 8 ++++++-- dascore/proc/coords.py | 2 +- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py index c57f54cb8..1d3cdcc1f 100644 --- a/dascore/core/coordmanager.py +++ b/dascore/core/coordmanager.py @@ -83,10 +83,18 @@ ) MaybeArray = ArrayLike | np.ndarray | None -CoordManagerInput = Mapping[ - str, - BaseCoord | np.ndarray | tuple[str | tuple[str, ...], BaseCoord | np.ndarray], -] + +# What a coord map may hold. Any sequence works as data -- a list or range is +# converted like an array -- except a bare tuple, which is reserved for the +# (dimension, data) form below. Mapping rather than dict so a caller's +# narrower value type still matches. +CoordInput = ( + BaseCoord + | np.ndarray + | Sequence[Any] + | tuple[str | tuple[str, ...], BaseCoord | np.ndarray | Sequence[Any]] +) +CoordManagerInput = Mapping[str, CoordInput] def _ensure_1d_coord(coord, coord_name: str): diff --git a/dascore/io/core.py b/dascore/io/core.py index 3510f0d66..de2e8ca88 100644 --- a/dascore/io/core.py +++ b/dascore/io/core.py @@ -219,8 +219,10 @@ def _scan_payload_to_summary( payload: ScanPayload | Mapping[str, Any], *, source_path: str | Path | UPath | None = None, - source_format: str | None = None, - source_version: str | None = None, + # PatchSummary stores these as plain strings and its validator maps a + # missing value to "", so default to what it would normalize None to. + source_format: str = "", + source_version: str = "", source_patch_id: str | None = None, ) -> PatchSummary: """Convert one structured FiberIO scan payload into a PatchSummary.""" diff --git a/dascore/proc/basic.py b/dascore/proc/basic.py index 5a6dbe607..20bc32f1f 100644 --- a/dascore/proc/basic.py +++ b/dascore/proc/basic.py @@ -12,7 +12,11 @@ from dascore.compat import array from dascore.constants import PatchType from dascore.core.attrs import PatchAttrs -from dascore.core.coordmanager import CoordManager, get_coord_manager +from dascore.core.coordmanager import ( + CoordManager, + CoordManagerInput, + get_coord_manager, +) from dascore.core.coords import get_coord from dascore.exceptions import ParameterError from dascore.utils.array import _apply_binary_ufunc @@ -200,7 +204,7 @@ def bool_patch(self: PatchType): def update( self: PatchType, data: ArrayLike | np.ndarray | None = None, - coords: dict[str | Sequence[str], ArrayLike] | CoordManager | None = None, + coords: CoordManagerInput | CoordManager | None = None, dims: Sequence[str] | None = None, attrs: Mapping | PatchAttrs | None = None, ) -> PatchType: diff --git a/dascore/proc/coords.py b/dascore/proc/coords.py index 72b79e8bd..aabe2f12d 100644 --- a/dascore/proc/coords.py +++ b/dascore/proc/coords.py @@ -260,7 +260,7 @@ def update_coords(self: PatchType, **kwargs) -> PatchType: @patch_function() -def drop_coords(self: PatchType, *coords: str | Collection[str]) -> PatchType: +def drop_coords(self: PatchType, *coords: str) -> PatchType: """ Update the coordinates of a patch. From 6f1f2622298082f304eb85785e4564a0a65419bc Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:12:56 +0200 Subject: [PATCH 3/8] Fix FrozenDict.new for non-string keys and validate_call config FrozenDict.new rebuilt the mapping with `self.__class__(**contents)`, which raises `TypeError: keywords must be strings` for any key that is not a valid identifier. It now passes the mapping positionally. pydantic.validate_call declares a ConfigDict, so build one rather than a plain dict. --- dascore/utils/mapping.py | 4 +++- dascore/utils/patch.py | 2 +- tests/test_utils/test_mapping_utils.py | 5 +++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/dascore/utils/mapping.py b/dascore/utils/mapping.py index 170e9f46b..900044197 100644 --- a/dascore/utils/mapping.py +++ b/dascore/utils/mapping.py @@ -41,9 +41,11 @@ def __contains__(self, key: object) -> bool: def new(self, **kwargs): """Copy the contents and update with new values.""" + # Passed as a mapping rather than splatted so keys that are not + # strings survive the round trip. contents = dict(self._dict) contents.update(kwargs) - return self.__class__(**contents) + return self.__class__(contents) def __iter__(self) -> Iterator[K]: return iter(self._dict) diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index 37dc23472..df7e6bc2f 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -301,7 +301,7 @@ def patch_function( def _wrapper(func): if validate_call: - config = dict(arbitrary_types_allowed=True) + config = pydantic.ConfigDict(arbitrary_types_allowed=True) func = pydantic.validate_call(config=config)(func) @functools.wraps(func) diff --git a/tests/test_utils/test_mapping_utils.py b/tests/test_utils/test_mapping_utils.py index bb3621bd0..bf31c5677 100644 --- a/tests/test_utils/test_mapping_utils.py +++ b/tests/test_utils/test_mapping_utils.py @@ -85,3 +85,8 @@ def test_new(self, frozen_dict): """Ensure new values can be added to the dict.""" out = frozen_dict.new(bob=10) assert out["bob"] == 10 + + def test_new_keeps_non_string_keys(self): + """Keys that cannot be keyword arguments must survive new.""" + froz = FrozenDict({1: "a", 2: "b"}) + assert dict(froz.new()) == {1: "a", 2: "b"} From 23ff0c2b16dea673ce71c5005e76157f4eeaa4f5 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:14:55 +0200 Subject: [PATCH 4/8] Widen two annotations that were narrower than their callers is_netcdf4_file and get_cf_version declared h5py.File but receive the managed handle a FiberIO caster produces, and their tests already pass plain duck types. Both read only `attrs`, so that is what they now ask for. get_patch_names forwards its input straight to scan_to_df, so it accepts everything that scans -- including the list of patches the DASDAE writer hands it. --- dascore/io/netcdf/utils.py | 18 ++++++++++++++++-- dascore/utils/patch.py | 10 ++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/dascore/io/netcdf/utils.py b/dascore/io/netcdf/utils.py index 733086aa2..c3f7898b6 100644 --- a/dascore/io/netcdf/utils.py +++ b/dascore/io/netcdf/utils.py @@ -2,6 +2,9 @@ from __future__ import annotations +from collections.abc import Mapping +from typing import Any, Protocol + import h5py import numpy as np @@ -32,7 +35,18 @@ def parse_cf_version(cf_version: str) -> tuple[int, int]: return major, minor -def is_netcdf4_file(h5file: h5py.File) -> bool: +class _HasAttrs(Protocol): + """Anything carrying HDF5-style attrs. + + The two checks below only read `attrs`, and they are handed the managed + handle a FiberIO caster produces rather than an `h5py.File` proper. + """ + + @property + def attrs(self) -> Mapping[str, Any]: ... + + +def is_netcdf4_file(h5file: _HasAttrs) -> bool: """Return True when an HDF5 file exposes strong NetCDF/CF markers.""" try: if "_NCProperties" in h5file.attrs: @@ -45,7 +59,7 @@ def is_netcdf4_file(h5file: h5py.File) -> bool: return False -def get_cf_version(h5file: h5py.File) -> str | None: +def get_cf_version(h5file: _HasAttrs) -> str | None: """Extract the CF convention version string from a NetCDF file.""" conventions = h5file.attrs.get("Conventions", "") if isinstance(conventions, bytes): diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index df7e6bc2f..983e704b1 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -8,7 +8,7 @@ import warnings from collections import namedtuple from collections.abc import Callable, Mapping, Sequence -from typing import Any, Literal, Protocol, cast, overload +from typing import TYPE_CHECKING, Any, Literal, Protocol, cast, overload import numpy as np import pandas as pd @@ -49,6 +49,10 @@ from dascore.utils.paths import is_memory_uri from dascore.utils.time import to_float +if TYPE_CHECKING: + # Imported lazily; dascore.io.core imports this module. + from dascore.io.core import ScanInput + attr_type = dict[str, Any] | str | Sequence[str] | None _DimAxisValue = namedtuple("_DimAxisValue", ["dim", "axis", "value"]) @@ -551,7 +555,9 @@ def get_start_stop_step(patch: PatchType, dim): def get_patch_names( - patch_data: pd.DataFrame | dc.Patch | dc.BaseSpool, + # Forwarded straight to scan_to_df, so anything it scans works here, + # including a plain list of patches. + patch_data: pd.DataFrame | ScanInput, prefix="DAS", attrs=("network", "station", "tag"), coords=("time",), From 85c10f62eb99cba3c54dc05166935e5bb4351911 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:19:04 +0200 Subject: [PATCH 5/8] Make two narrowings visible to the type checker patch_function handles the bare-decorator form by re-entering itself, so required_dims is only ever a callable on that path. Doing the check before the wrapper is built rather than after leaves the rest of the function seeing the tuple of dimension names it actually gets. The assembly loop's first pass sets axis and dims alongside buffer; assert all three rather than just the one. --- dascore/utils/patch.py | 7 ++++--- dascore/utils/patch_assembly.py | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index 983e704b1..a1e7ee910 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -302,6 +302,10 @@ def patch_function( as dc at the top of the file where the patch function is defined so the forward refs can be resolved properly for type checking. """ + # Handled before the wrapper is built so the rest of this function sees + # required_dims as the tuple of dimension names it is everywhere else. + if callable(required_dims): # the decorator is used without parens + return patch_function()(required_dims) def _wrapper(func): if validate_call: @@ -342,9 +346,6 @@ def _func(patch, *args, **kwargs): return patch_func - if callable(required_dims): # the decorator is used without parens - return patch_function()(required_dims) - return _wrapper diff --git a/dascore/utils/patch_assembly.py b/dascore/utils/patch_assembly.py index 643e66707..8b138182a 100644 --- a/dascore/utils/patch_assembly.py +++ b/dascore/utils/patch_assembly.py @@ -224,7 +224,10 @@ def _merge_patches_streaming(self, joined, df_dict_list, merge_dim, samples): coords.append(patch.coords) attrs.append(patch.attrs) summaries.append(patch.coords._get_dim_summary()) - assert buffer is not None # allocated on the first pass of the loop + # All set on the first pass of the loop, which always runs. + assert buffer is not None + assert axis is not None + assert dims is not None if offset != buffer.shape[axis]: # over-estimated; trim excess. buffer = buffer[broadcast_for_index(buffer.ndim, axis, slice(0, offset))] # Ensure the loaded patches only vary along the expected dimension, From 9748f798d95856c37f035c26a0a9621789f338f4 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:21:45 +0200 Subject: [PATCH 6/8] Give the catalog view sentinel a type _KEEP was a bare object(), so _view's order and ids parameters were inferred as object and neither could be passed on to the constructor. A dedicated class carries the same meaning and lets an isinstance check narrow the parameter back to the spec type. --- dascore/io/index/catalog.py | 23 ++++++++++++++++++----- dascore/io/netcdf/utils.py | 1 - dascore/proc/coords.py | 2 -- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dascore/io/index/catalog.py b/dascore/io/index/catalog.py index 181ebcb36..dee725bb0 100644 --- a/dascore/io/index/catalog.py +++ b/dascore/io/index/catalog.py @@ -474,8 +474,15 @@ def clear(self) -> None: self.value, self.revision = None, -1 -# sentinel: _view keeps the current order/ids spec unless told otherwise -_KEEP = object() +class _Keep: + """Sentinel: _view keeps the current order/ids spec unless told otherwise. + + A dedicated class rather than a bare object so that testing against it + narrows the parameter to the spec type it otherwise holds. + """ + + +_KEEP = _Keep() class PatchCatalog: @@ -702,7 +709,13 @@ def __getstate__(self) -> dict: state["resolver"] = _membership_resolver(resolver, keep, paths) return state - def _view(self, queries, residuals, order=_KEEP, ids=_KEEP) -> PatchCatalog: + def _view( + self, + queries, + residuals, + order: tuple | _Keep | None = _KEEP, + ids: tuple | _Keep | None = _KEEP, + ) -> PatchCatalog: out = PatchCatalog( backend=self.backend, resolver=self.resolver, @@ -710,8 +723,8 @@ def _view(self, queries, residuals, order=_KEEP, ids=_KEEP) -> PatchCatalog: queries=queries, residuals=residuals, revision=self._revision, - order=self._order if order is _KEEP else order, - ids=self._ids if ids is _KEEP else ids, + order=self._order if isinstance(order, _Keep) else order, + ids=self._ids if isinstance(ids, _Keep) else ids, default_order=self._default_order, ) return out diff --git a/dascore/io/netcdf/utils.py b/dascore/io/netcdf/utils.py index c3f7898b6..d131a1ee5 100644 --- a/dascore/io/netcdf/utils.py +++ b/dascore/io/netcdf/utils.py @@ -5,7 +5,6 @@ from collections.abc import Mapping from typing import Any, Protocol -import h5py import numpy as np import dascore as dc diff --git a/dascore/proc/coords.py b/dascore/proc/coords.py index aabe2f12d..b63347899 100644 --- a/dascore/proc/coords.py +++ b/dascore/proc/coords.py @@ -2,8 +2,6 @@ from __future__ import annotations -from collections.abc import Collection - import numpy as np import pandas as pd from scipy.interpolate import interp1d From 923105f5ac961efd90a9c69ba607254d9f1aebb7 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Thu, 6 Aug 2026 13:42:40 +0200 Subject: [PATCH 7/8] Make drop_coords accept a sequence of names Both Patch.drop_coords and CoordManager.drop_coords advertised Collection[str] but no collection form worked: a list or set raised TypeError on the set intersection and a tuple silently dropped nothing. iterate was applied to the varargs tuple as a whole, which is already iterable, so anything passed as a sequence survived as a single unhashable element. Applying it per argument flattens as intended and makes the documented signature true at both levels. --- dascore/core/coordmanager.py | 11 +++++++---- dascore/proc/coords.py | 14 ++++++++++---- tests/test_core/test_coordmanager.py | 7 +++++++ tests/test_proc/test_proc_coords.py | 17 +++++++++++++++++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py index 1d3cdcc1f..87a074d9d 100644 --- a/dascore/core/coordmanager.py +++ b/dascore/core/coordmanager.py @@ -42,7 +42,7 @@ from __future__ import annotations from collections import defaultdict -from collections.abc import Mapping, Sequence +from collections.abc import Collection, Mapping, Sequence from itertools import zip_longest from types import EllipsisType from typing import Annotated, Any @@ -456,7 +456,7 @@ def new(self, dims=None, coord_map=None, dim_map=None, **kwargs) -> Self: def drop_coords( self, - *coords: str, + *coords: str | Collection[str], array: MaybeArray = None, ) -> tuple[Self, MaybeArray]: """ @@ -468,10 +468,13 @@ def drop_coords( Parameters ---------- *coords - The name of the coordinate or dimension. + The name of the coordinate or dimension, or a sequence of them. """ dim_drop_list = [] - coords_to_drop = {x for x in iterate(coords)} + # iterate is applied per argument; the varargs tuple is already + # iterable, so flattening it as a whole would leave any sequence + # passed in as a single unhashable element. + coords_to_drop = {x for coord in coords for x in iterate(coord)} # If there are either no coords to drop or this cm doesn't have them. if not coords_to_drop or not (set(self.coord_map) & coords_to_drop): return self, array diff --git a/dascore/proc/coords.py b/dascore/proc/coords.py index b63347899..935d73709 100644 --- a/dascore/proc/coords.py +++ b/dascore/proc/coords.py @@ -2,6 +2,8 @@ from __future__ import annotations +from collections.abc import Collection + import numpy as np import pandas as pd from scipy.interpolate import interp1d @@ -258,7 +260,7 @@ def update_coords(self: PatchType, **kwargs) -> PatchType: @patch_function() -def drop_coords(self: PatchType, *coords: str) -> PatchType: +def drop_coords(self: PatchType, *coords: str | Collection[str]) -> PatchType: """ Update the coordinates of a patch. @@ -267,7 +269,8 @@ def drop_coords(self: PatchType, *coords: str) -> PatchType: Parameters ---------- *coords - One or more coordinates to drop. + One or more coordinates to drop. Each can be a coordinate name or + a sequence of them. Examples -------- @@ -276,11 +279,14 @@ def drop_coords(self: PatchType, *coords: str) -> PatchType: >>> pa = dc.get_example_patch("random_patch_with_lat_lon") >>> # Drop non-dimensional coordinate latitude >>> pa_no_lat = pa.drop_coords("latitude") + >>> # A sequence of names works as well. + >>> pa_no_lat = pa.drop_coords(["latitude"]) """ - if dim_coords := set(coords) & set(self.dims): + names = {x for coord in coords for x in iterate(coord)} + if dim_coords := names & set(self.dims): msg = f"Cannot drop dimensional coordinates: {dim_coords}" raise ParameterError(msg) - new_coord, data = self.coords.drop_coords(*coords, array=self.data) + new_coord, data = self.coords.drop_coords(*names, array=self.data) return self.new(coords=new_coord, dims=new_coord.dims, data=data) diff --git a/tests/test_core/test_coordmanager.py b/tests/test_core/test_coordmanager.py index 1c271c13c..653088161 100644 --- a/tests/test_core/test_coordmanager.py +++ b/tests/test_core/test_coordmanager.py @@ -339,6 +339,13 @@ def test_drop_doesnt_have_coord(self, cm_multidim): out, _ = cm_multidim.drop_coords("bob") assert out == cm_multidim + @pytest.mark.parametrize("wrap", [list, tuple, set]) + def test_drop_sequence(self, cm_multidim, wrap): + """A sequence of names should drop each one.""" + dim = "distance" + coords, _ = cm_multidim.drop_coords(wrap([dim])) + assert dim not in coords.dims + def test_trims_array(self, cm_multidim): """Trying to drop a dim that doesnt exist should just return.""" array = np.ones(cm_multidim.shape) diff --git a/tests/test_proc/test_proc_coords.py b/tests/test_proc/test_proc_coords.py index 4b2e31160..0362cf059 100644 --- a/tests/test_proc/test_proc_coords.py +++ b/tests/test_proc/test_proc_coords.py @@ -154,6 +154,23 @@ def test_drop_dim_raises(self, random_patch): with pytest.raises(ParameterError, match=msg): random_patch.drop_coords("time") + @pytest.mark.parametrize("form", [["latitude"], ("latitude",), {"latitude"}]) + def test_drop_sequence(self, random_patch_with_lat_lon, form): + """A sequence of names should drop each one.""" + out = random_patch_with_lat_lon.drop_coords(form) + assert "latitude" not in out.coords.coord_map + + def test_drop_mixed_args(self, random_patch_with_lat_lon): + """Names and sequences of names should be usable together.""" + out = random_patch_with_lat_lon.drop_coords("latitude", ["longitude"]) + assert not {"latitude", "longitude"} & set(out.coords.coord_map) + + def test_drop_dim_in_sequence_raises(self, random_patch): + """A dimension inside a sequence should raise like a bare one.""" + msg = "Cannot drop dimensional coordinates" + with pytest.raises(ParameterError, match=msg): + random_patch.drop_coords(["time"]) + class TestCoordsFromDf: """Tests for attaching coordinate(s) to a patch.""" From 54548af4aff6142b7c3790d7fa995f1d40adacbe Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Fri, 7 Aug 2026 12:15:17 +0200 Subject: [PATCH 8/8] Correct three annotations the adversarial review disproved CoordManagerInput claimed more than it delivered. Its Sequence[Any] arm subsumed every tuple, so the "bare tuple is reserved for the (dim, data) form" comment was false and the nested tuple arm rejected nothing, while the union still turned away int and Quantity values (partial coords) that first-party callers pass. It now matches the Patch constructor's coords. get_patch_names referenced io.core.ScanInput behind TYPE_CHECKING. The import cycle is real, but the annotation then failed to resolve at runtime, and the docs renderer falls back all-or-nothing to raw strings for a signature when any name is undefined. Spelled out instead. drop_coords accepts any iterable, not just a sized collection, since iterate branches on Iterable; a generator worked but did not type check. The drop_coords tests only asserted the named coords were gone, so an implementation that dropped every coord passed them. They now compare against the bare-name call. --- dascore/core/coordmanager.py | 22 +++++++++------------- dascore/proc/coords.py | 4 ++-- dascore/units.py | 9 ++++++--- dascore/utils/patch.py | 15 +++++++-------- docs/changelog.qmd | 1 + tests/test_core/test_coordmanager.py | 7 +++++-- tests/test_proc/test_proc_coords.py | 20 +++++++++++++++----- tests/test_units.py | 7 +++++++ tests/test_utils/test_patch_utils.py | 9 +++++++++ 9 files changed, 61 insertions(+), 33 deletions(-) diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py index 87a074d9d..40d1d008c 100644 --- a/dascore/core/coordmanager.py +++ b/dascore/core/coordmanager.py @@ -42,7 +42,7 @@ from __future__ import annotations from collections import defaultdict -from collections.abc import Collection, Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from itertools import zip_longest from types import EllipsisType from typing import Annotated, Any @@ -84,17 +84,13 @@ MaybeArray = ArrayLike | np.ndarray | None -# What a coord map may hold. Any sequence works as data -- a list or range is -# converted like an array -- except a bare tuple, which is reserved for the -# (dimension, data) form below. Mapping rather than dict so a caller's -# narrower value type still matches. -CoordInput = ( - BaseCoord - | np.ndarray - | Sequence[Any] - | tuple[str | tuple[str, ...], BaseCoord | np.ndarray | Sequence[Any]] -) -CoordManagerInput = Mapping[str, CoordInput] +# What a coord map may hold, kept identical to the Patch constructor's coords. +# The value stays Any on purpose: get_coord_manager also accepts an int or a +# Quantity (a partial coord), a mapping of start/stop/step, and a +# (dimension, data) tuple, and every union narrow enough to be worth writing +# rejected one of those first-party forms. Mapping rather than dict so a +# caller's narrower value type still matches. +CoordManagerInput = Mapping[str, Any] def _ensure_1d_coord(coord, coord_name: str): @@ -456,7 +452,7 @@ def new(self, dims=None, coord_map=None, dim_map=None, **kwargs) -> Self: def drop_coords( self, - *coords: str | Collection[str], + *coords: str | Iterable[str], array: MaybeArray = None, ) -> tuple[Self, MaybeArray]: """ diff --git a/dascore/proc/coords.py b/dascore/proc/coords.py index 935d73709..bbb3ce953 100644 --- a/dascore/proc/coords.py +++ b/dascore/proc/coords.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Collection +from collections.abc import Iterable import numpy as np import pandas as pd @@ -260,7 +260,7 @@ def update_coords(self: PatchType, **kwargs) -> PatchType: @patch_function() -def drop_coords(self: PatchType, *coords: str | Collection[str]) -> PatchType: +def drop_coords(self: PatchType, *coords: str | Iterable[str]) -> PatchType: """ Update the coordinates of a patch. diff --git a/dascore/units.py b/dascore/units.py index c5479bd6b..5e3261e24 100644 --- a/dascore/units.py +++ b/dascore/units.py @@ -531,8 +531,11 @@ def __getattr__(name: str) -> Quantity: from dascore.units import get_quantity m = get_quantity("m") - A name is always a non-empty identifier here, so this either resolves - to a quantity or raises UndefinedUnitError; it never returns None the - way get_quantity does for None or an empty string. + Any non-empty name either resolves to a quantity or raises + UndefinedUnitError, so the cast holds. The empty string is the one input + get_quantity maps to None, and attribute access is the right place to + reject it. """ + if not name: + raise AttributeError(name) return cast("Quantity", get_quantity(name)) diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index a1e7ee910..b9049a1d7 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -7,8 +7,8 @@ import sys import warnings from collections import namedtuple -from collections.abc import Callable, Mapping, Sequence -from typing import TYPE_CHECKING, Any, Literal, Protocol, cast, overload +from collections.abc import Callable, Iterable, Mapping, Sequence +from typing import Any, Literal, Protocol, cast, overload import numpy as np import pandas as pd @@ -49,10 +49,6 @@ from dascore.utils.paths import is_memory_uri from dascore.utils.time import to_float -if TYPE_CHECKING: - # Imported lazily; dascore.io.core imports this module. - from dascore.io.core import ScanInput - attr_type = dict[str, Any] | str | Sequence[str] | None _DimAxisValue = namedtuple("_DimAxisValue", ["dim", "axis", "value"]) @@ -557,8 +553,11 @@ def get_start_stop_step(patch: PatchType, dim): def get_patch_names( # Forwarded straight to scan_to_df, so anything it scans works here, - # including a plain list of patches. - patch_data: pd.DataFrame | ScanInput, + # including a plain list of patches. Spelled out rather than reusing + # io.core.ScanInput: importing that is circular, and hiding it behind + # TYPE_CHECKING leaves the annotation unresolvable at runtime, which + # breaks get_type_hints and the API doc renderer. + patch_data: pd.DataFrame | dc.Patch | dc.BaseSpool | Iterable[dc.Patch], prefix="DAS", attrs=("network", "station", "tag"), coords=("time",), diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 711461c83..6b4d909cb 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 +- `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. - **The `dascore.io.sintela_binary` module is removed (no alias).** Both Sintela readers now live in `dascore.io.sintela`, which also provides the new protobuf reader; use `from dascore.io.sintela import SintelaBinaryV3`. Reading Sintela binary files through `dc.read`/`dc.spool`/`dc.scan` is unaffected — only the direct module import path changed. diff --git a/tests/test_core/test_coordmanager.py b/tests/test_core/test_coordmanager.py index 653088161..cdd8688a2 100644 --- a/tests/test_core/test_coordmanager.py +++ b/tests/test_core/test_coordmanager.py @@ -339,12 +339,15 @@ def test_drop_doesnt_have_coord(self, cm_multidim): out, _ = cm_multidim.drop_coords("bob") assert out == cm_multidim - @pytest.mark.parametrize("wrap", [list, tuple, set]) + @pytest.mark.parametrize("wrap", [list, tuple, set, iter]) def test_drop_sequence(self, cm_multidim, wrap): - """A sequence of names should drop each one.""" + """A sequence of names should behave exactly like the bare name.""" dim = "distance" coords, _ = cm_multidim.drop_coords(wrap([dim])) + expected, _ = cm_multidim.drop_coords(dim) assert dim not in coords.dims + # Compared to the bare-name call so that dropping too much fails too. + assert coords == expected def test_trims_array(self, cm_multidim): """Trying to drop a dim that doesnt exist should just return.""" diff --git a/tests/test_proc/test_proc_coords.py b/tests/test_proc/test_proc_coords.py index 0362cf059..75b37d30b 100644 --- a/tests/test_proc/test_proc_coords.py +++ b/tests/test_proc/test_proc_coords.py @@ -154,16 +154,26 @@ def test_drop_dim_raises(self, random_patch): with pytest.raises(ParameterError, match=msg): random_patch.drop_coords("time") - @pytest.mark.parametrize("form", [["latitude"], ("latitude",), {"latitude"}]) + @pytest.mark.parametrize( + "form", [["latitude"], ("latitude",), {"latitude"}, iter(["latitude"])] + ) def test_drop_sequence(self, random_patch_with_lat_lon, form): - """A sequence of names should drop each one.""" - out = random_patch_with_lat_lon.drop_coords(form) + """A sequence of names should behave exactly like the bare name.""" + patch = random_patch_with_lat_lon + out = patch.drop_coords(form) + expected = patch.drop_coords("latitude") assert "latitude" not in out.coords.coord_map + # Compared to the bare-name call so that dropping too much fails too. + assert set(out.coords.coord_map) == set(expected.coords.coord_map) def test_drop_mixed_args(self, random_patch_with_lat_lon): """Names and sequences of names should be usable together.""" - out = random_patch_with_lat_lon.drop_coords("latitude", ["longitude"]) - assert not {"latitude", "longitude"} & set(out.coords.coord_map) + patch = random_patch_with_lat_lon + out = patch.drop_coords("latitude", ["longitude"]) + dropped = {"latitude", "longitude"} + assert not dropped & set(out.coords.coord_map) + # Everything else has to survive. + assert set(out.coords.coord_map) == set(patch.coords.coord_map) - dropped def test_drop_dim_in_sequence_raises(self, random_patch): """A dimension inside a sequence should raise like a bare one.""" diff --git a/tests/test_units.py b/tests/test_units.py index 6e32a887b..ac4a5b58e 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -222,6 +222,13 @@ def test_bad_import_error_msg(self): with pytest.raises(ImportError): from dascore.utils import bob # noqa + def test_empty_name_raises(self): + """The empty string is the one name get_quantity maps to None.""" + import dascore.units + + with pytest.raises(AttributeError): + getattr(dascore.units, "") + class TestGetFilterUnits: """Tests for getting units that can be used for filtering.""" diff --git a/tests/test_utils/test_patch_utils.py b/tests/test_utils/test_patch_utils.py index ec859be66..394192caa 100644 --- a/tests/test_utils/test_patch_utils.py +++ b/tests/test_utils/test_patch_utils.py @@ -838,6 +838,15 @@ def test_empty(self): out = get_patch_names([]) assert isinstance(out, pd.Series) + def test_annotations_resolve(self): + """ + The docs renderer resolves annotations at runtime and falls back to + raw strings for the whole signature if any name is undefined. + """ + from typing import get_type_hints + + assert get_type_hints(get_patch_names) + def test_name_column_exists(self, random_spool): """If the name or path field already exist this should be used.""" df = random_spool.get_contents().assign(name=lambda x: np.arange(len(x)))