diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py index c57f54cb8..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 Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from itertools import zip_longest from types import EllipsisType from typing import Annotated, Any @@ -83,10 +83,14 @@ ) MaybeArray = ArrayLike | np.ndarray | None -CoordManagerInput = Mapping[ - str, - BaseCoord | np.ndarray | tuple[str | tuple[str, ...], BaseCoord | np.ndarray], -] + +# 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): @@ -448,7 +452,7 @@ def new(self, dims=None, coord_map=None, dim_map=None, **kwargs) -> Self: def drop_coords( self, - *coords: str, + *coords: str | Iterable[str], array: MaybeArray = None, ) -> tuple[Self, MaybeArray]: """ @@ -460,10 +464,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/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/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 733086aa2..d131a1ee5 100644 --- a/dascore/io/netcdf/utils.py +++ b/dascore/io/netcdf/utils.py @@ -2,7 +2,9 @@ from __future__ import annotations -import h5py +from collections.abc import Mapping +from typing import Any, Protocol + import numpy as np import dascore as dc @@ -32,7 +34,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 +58,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/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..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. @@ -269,7 +269,8 @@ def drop_coords(self: PatchType, *coords: str | Collection[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 -------- @@ -278,11 +279,14 @@ def drop_coords(self: PatchType, *coords: str | Collection[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/dascore/units.py b/dascore/units.py index 21ab5dfcc..5e3261e24 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,12 @@ def __getattr__(name): is the same as from dascore.units import get_quantity m = get_quantity("m") + + 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. """ - return get_quantity(name) + if not name: + raise AttributeError(name) + return cast("Quantity", get_quantity(name)) 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..b9049a1d7 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -7,7 +7,7 @@ import sys import warnings from collections import namedtuple -from collections.abc import Callable, Mapping, Sequence +from collections.abc import Callable, Iterable, Mapping, Sequence from typing import Any, Literal, Protocol, cast, overload import numpy as np @@ -298,10 +298,14 @@ 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: - config = dict(arbitrary_types_allowed=True) + config = pydantic.ConfigDict(arbitrary_types_allowed=True) func = pydantic.validate_call(config=config)(func) @functools.wraps(func) @@ -338,9 +342,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 @@ -551,7 +552,12 @@ 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. 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/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, 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 1c271c13c..cdd8688a2 100644 --- a/tests/test_core/test_coordmanager.py +++ b/tests/test_core/test_coordmanager.py @@ -339,6 +339,16 @@ 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, iter]) + def test_drop_sequence(self, cm_multidim, wrap): + """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.""" 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..75b37d30b 100644 --- a/tests/test_proc/test_proc_coords.py +++ b/tests/test_proc/test_proc_coords.py @@ -154,6 +154,33 @@ 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"}, iter(["latitude"])] + ) + def test_drop_sequence(self, random_patch_with_lat_lon, 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.""" + 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.""" + 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.""" 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_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"} 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)))