Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions dascore/core/coordmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from collections.abc import Iterable, Mapping, Sequence
from itertools import zip_longest
from types import EllipsisType
from typing import Annotated, Any
from typing import Annotated, Any, cast

import numpy as np
from pydantic import field_validator, model_validator
Expand Down Expand Up @@ -310,7 +310,12 @@ def _divide_kwargs(kwargs):
out[coord_name] = (coord_dims, coord.update(**{attr: value}))

dims = tuple(x for x in dims if x not in coord_to_drop)
return get_coord_manager(out, dims=dims)
# Cast because the factory normalizes the coord mapping in ways the
# class constructor does not, so this cannot go through
# self.__class__ the way drop_coords does. Exact for CoordManager
# itself; a subclass would already lose its type here, which is a
# limitation of the factory rather than of this annotation.
return cast("Self", get_coord_manager(out, dims=dims))

# we need this here to maintain backwards compatibility
update_coords = update
Expand Down Expand Up @@ -509,14 +514,14 @@ def disassociate_coord(self, *coord: str) -> Self:
new = {x: (None, self.coord_map[x]) for x in coord}
return self.drop_coords(*coord)[0].update(**new)

def drop_disassociated_coords(self) -> Self:
def drop_disassociated_coords(self) -> tuple[Self, MaybeArray]:
"""Drop all coordinates not associated with a dimension."""
cmap = self.coord_map
dim_map = self.dim_map
no_dim_coords = [x for x in cmap if dim_map[x] == ()]
return self.drop_coords(*no_dim_coords)

def drop_private_coords(self, array=None) -> Self:
def drop_private_coords(self, array=None) -> tuple[Self, MaybeArray]:
"""Drop all coordinates whose name begin with an underscore."""
cmap = self.coord_map
private = tuple(x for x in cmap.keys() if x.startswith("_"))
Expand Down
23 changes: 17 additions & 6 deletions dascore/core/coords.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from functools import cache
from operator import gt, lt
from types import EllipsisType
from typing import TYPE_CHECKING, Any, Literal, overload
from typing import TYPE_CHECKING, Any, Literal, cast, overload

import numpy as np
import pandas as pd
Expand Down Expand Up @@ -512,6 +512,10 @@ def __getitem__(self, item: int | np.integer) -> Any: ...
def __getitem__(self, item: slice | np.ndarray) -> Self: ...

@abc.abstractmethod
# Left unannotated on purpose. An int index yields a bare value, so the
# honest return contains Any, and Any absorbs everything -- annotating it
# would not let the checker verify the overloads above, only look as if
# it did.
def __getitem__(self, item):
"""Index the coord; slices return a new coord, int indices a value."""

Expand Down Expand Up @@ -613,8 +617,8 @@ def max(self):
return self._max()

@property
def unit_str(self) -> str:
"""Return a unit string."""
def unit_str(self) -> str | None:
"""Return a unit string, or None for a coord carrying no units."""
return get_quantity_str(self.units)

@abc.abstractmethod
Expand All @@ -640,7 +644,9 @@ def ndim(self) -> int:
@property
def size(self) -> int:
"""Return the size of the coordinate data."""
return np.prod(self.shape)
# math rather than np.prod: the shape is a tuple of ints, and numpy
# hands back an np.int64 (or a float 1.0 for the empty shape).
return math.prod(self.shape)

@property
def evenly_sampled(self) -> bool:
Expand Down Expand Up @@ -1033,10 +1039,13 @@ def _get_index(self, value, forward=True):

def get_next_index(
self, value, samples=False, allow_out_of_bounds=False, relative=False
) -> int:
) -> np.ndarray | np.integer:
"""
Get the index a value would have in a coordinate.

A sized value yields an array of indices; anything else yields a
single index, which is a numpy integer rather than a builtin int.

This returns the "next" rather than the closest, index if the exact
value is not contained by the index.

Expand Down Expand Up @@ -1328,7 +1337,9 @@ 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=(_validate_new_length(length),))
# A shape-only coord is always partial, so this really is Self; the
# factory's declared BaseCoord return is just wider than the case.
return cast("Self", get_coord(shape=(_validate_new_length(length),)))

def to_summary(self, dims=()) -> CoordSummary:
"""Get the summary info about the coord."""
Expand Down
20 changes: 11 additions & 9 deletions dascore/io/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,10 @@ def __init__(self, entry_point: str):
self._fiber_io_by_input_type: dict[str, set[FiberIO]] = {}
self._fiber_io_name_ver = set()
# Snapshots derived from the registry; cleared when it changes.
self._lookup_cache: dict[tuple, frozenset | tuple] = {}
# Kept as two dicts rather than one keyed by a discriminating
# prefix so each stays a single value type.
self._input_type_cache: dict[str, frozenset[FiberIO]] = {}
self._prioritized_cache: dict[str, tuple[FiberIO, ...]] = {}
Comment on lines +431 to +434

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include None in both cache key types.

yield_fiberio accepts input_type: str | None and can pass None to both cache-backed helpers. Both caches can therefore contain a None key, but their declarations allow only str. Change both key types to str | None.

Proposed type correction
-        self._input_type_cache: dict[str, frozenset[FiberIO]] = {}
-        self._prioritized_cache: dict[str, tuple[FiberIO, ...]] = {}
+        self._input_type_cache: dict[str | None, frozenset[FiberIO]] = {}
+        self._prioritized_cache: dict[str | None, tuple[FiberIO, ...]] = {}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Kept as two dicts rather than one keyed by a discriminating
# prefix so each stays a single value type.
self._input_type_cache: dict[str, frozenset[FiberIO]] = {}
self._prioritized_cache: dict[str, tuple[FiberIO, ...]] = {}
# Kept as two dicts rather than one keyed by a discriminating
# prefix so each stays a single value type.
self._input_type_cache: dict[str | None, frozenset[FiberIO]] = {}
self._prioritized_cache: dict[str | None, tuple[FiberIO, ...]] = {}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dascore/io/core.py` around lines 431 - 434, Update the key type annotations
for _input_type_cache and _prioritized_cache to str | None, preserving their
existing value types and cache behavior so None keys accepted by yield_fiberio
and its helpers are represented correctly.


def __getstate__(self) -> dict:
"""Return copy/pickle state without the process-local lock."""
Expand Down Expand Up @@ -467,20 +470,18 @@ def unloaded_formats(self) -> list[str]:
@_locked("_lock")
def _get_fiber_io_by_input_type(self, input_type) -> frozenset[FiberIO]:
"""Get a set of FiberIO instances that meet input type."""
key = ("input_type", input_type)
if (cached := self._lookup_cache.get(key)) is None:
if (cached := self._input_type_cache.get(input_type)) is None:
if (out := self._fiber_io_by_input_type.get(input_type)) is None:
out = set()
for input_set in self._fiber_io_by_input_type.values():
out |= input_set
cached = self._lookup_cache[key] = frozenset(out)
cached = self._input_type_cache[input_type] = frozenset(out)
return cached

@_locked("_lock")
def _get_prioritized_list(self, input_type="file") -> tuple[FiberIO, ...]:
"""Yield a prioritized list of fiber_ios."""
key = ("prioritized", input_type)
if (cached := self._lookup_cache.get(key)) is not None:
if (cached := self._prioritized_cache.get(input_type)) is not None:
return cached
# must load all plugins before getting list
self.load_plugins()
Expand All @@ -499,7 +500,7 @@ def _get_prioritized_list(self, input_type="file") -> tuple[FiberIO, ...]:
valid_fiberio_by_type = self._get_fiber_io_by_input_type(input_type)
out = tuple(x for x in maybe_ios if x in valid_fiberio_by_type)
# And return fiberIOs that much the input type.
self._lookup_cache[key] = out
self._prioritized_cache[input_type] = out
return out

def load_plugins(self, format: str | None = None):
Expand Down Expand Up @@ -566,7 +567,8 @@ def register_fiberio(self, fiberio: FiberIO):
self._fiber_io_by_input_type.setdefault(fiberio.input_type, set()).add(fiberio)
self._fiber_io_name_ver.add(id_tuple)
# Snapshots derived from the registry are now stale.
self._lookup_cache.clear()
self._input_type_cache.clear()
self._prioritized_cache.clear()

@cached_method
def get_fiberio(
Expand Down Expand Up @@ -1687,7 +1689,7 @@ def write(
file_version: str | None = None,
split: bool = False,
**kwargs,
) -> Path:
) -> path_types:
"""
Write a Patch or Spool to disk.

Expand Down
5 changes: 3 additions & 2 deletions dascore/io/dasvader/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,9 @@ def _is_dasvader_jld2(h5) -> bool:
return False
# Certain refs that all dasvader files have.
has_expected = EXPECTED.issubset(set(dtype_names))
# Data name can change.
has_data = DATA_NAMES & set(dtype_names)
# Data name can change. Coerced to a bool so an empty intersection
# returns False rather than the empty set the `and` would hand back.
has_data = bool(DATA_NAMES & set(dtype_names))
return has_data and has_expected


Expand Down
3 changes: 2 additions & 1 deletion dascore/io/febus/a1utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

from collections import namedtuple
from collections.abc import Iterator
from functools import cache

import numpy as np
Expand Down Expand Up @@ -289,7 +290,7 @@ def _get_febus_coord_manager(feb: _FebusSlice) -> CoordManager:
return cm


def _yield_attrs_coords(fi) -> tuple[dict, CoordManager]:
def _yield_attrs_coords(fi) -> Iterator[tuple[dict, CoordManager, _FebusSlice]]:
"""Scan a febus file, return metadata."""
febuses = _flatten_febus_info(fi)
for febus in febuses:
Expand Down
8 changes: 6 additions & 2 deletions dascore/io/index/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,12 @@ def _compatible_coord_units(
for other in query_units - {first}:
convert_units(1.0, to_units=first, from_units=other)
stored = {_normalize_unit(x) for x in rows.get("units", ())}
compatible = set()
for unit in stored - {None}:
compatible: set[str] = set()
for unit in stored:
# Skipped rather than differenced out so the unitless rows, which
# are handled by the check below, stay out of the result set.
if unit is None:
continue
try:
convert_units(1.0, to_units=unit, from_units=first)
except UnitError:
Expand Down
2 changes: 1 addition & 1 deletion dascore/io/netcdf/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
XDAS_PAYLOAD_VARIABLE = "__values__"


def get_xarray_data_var_name(dataset) -> str:
def get_xarray_data_var_name(dataset) -> str | None:
"""Return the main xarray data variable name."""
if "data" in dataset.data_vars:
return "data"
Expand Down
2 changes: 1 addition & 1 deletion dascore/io/prodml/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def scan(
self, resource: H5Reader, snap: bool = True, **kwargs
) -> list[ScanPayload]:
"""Scan a prodml file, return summary information about the file's contents."""
out = []
out: list[ScanPayload] = []
for attr, coords, source_patch_id in _yield_prodml_attrs_coords(
resource, snap=snap
):
Expand Down
4 changes: 2 additions & 2 deletions dascore/io/sintela/protobuf_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
from dascore.core.coordmanager import get_coord_manager
from dascore.core.coords import get_coord
from dascore.exceptions import InvalidFiberFileError
from dascore.io.core import _make_scan_payload
from dascore.io.core import ScanPayload, _make_scan_payload
from dascore.utils.misc import optional_import, suppress_warnings
from dascore.utils.models import DascoreBaseModel, PositiveFiniteFloat, PositiveInt

Expand Down Expand Up @@ -1049,7 +1049,7 @@ def read_payload(resource):
return _decode_family(parsed, meta)


def scan_payload(resource) -> list[dict[str, Any]]:
def scan_payload(resource) -> list[ScanPayload]:
"""Decode a Sintela protobuf file and return FiberIO scan payloads."""
records = _iter_envelope_records(resource, strict=True)
parsed, meta = _parse_records(records, scan_mode=True)
Expand Down
4 changes: 2 additions & 2 deletions dascore/proc/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def sobel_filter(
dim, mode, cval = _check_sobel_args(dim, mode, cval)
axis = patch.get_axis(dim)
out = ndimage.sobel(patch.data, axis=axis, mode=mode, cval=cval)
return dc.Patch(data=out, coords=patch.coords, attrs=patch.attrs, dims=patch.dims)
return patch.new(data=out)


def _create_size_and_axes(patch, kwargs, samples):
Expand Down Expand Up @@ -340,7 +340,7 @@ def notch_filter(patch: PatchType, q: float, **kwargs) -> PatchType:
raise FilterValueError(msg)
b, a = iirnotch(w0, Q=q, fs=sr)
data = filtfilt(b, a, data, axis=axis)
return dc.Patch(data=data, coords=patch.coords, attrs=patch.attrs, dims=patch.dims)
return patch.new(data=data)


@patch_function()
Expand Down
2 changes: 1 addition & 1 deletion dascore/transform/strain.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def velocity_to_strain_rate_edgeless(
data_units=new_data_units,
)

return dc.Patch(data=strain_rate, coords=new_coords, attrs=new_attrs)
return patch.new(data=strain_rate, coords=new_coords, attrs=new_attrs)


@patch_function()
Expand Down
12 changes: 9 additions & 3 deletions dascore/units.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,13 @@ def convert_units(
return (data * mult1 + add) * mult2


def assert_dtype_compatible_with_units(dtype, quantity) -> Quantity:
def assert_dtype_compatible_with_units(dtype, quantity) -> Quantity | None:
"""
Return quantity if it is compatible with dtype.

A quantity of None passes through for a non-time dtype and raises for
a time-like one, where seconds are the only allowable units.

If not raise [UnitError](`dascore.exceptions.UnitError`).
"""
if not dtype_time_like(dtype):
Expand Down Expand Up @@ -442,15 +445,18 @@ def quant_sequence_to_quant_array(sequence: Sequence[Quantity]) -> Quantity:
"""
if is_array(sequence):
# This is a numpy array, just return multiplied by quantity.
return sequence * get_quantity("dimensionless")
# Cast because numpy declares ndarray.__mul__ as returning an
# ndarray; pint's reflected __rmul__ is what actually runs and it
# yields a Quantity.
return cast("Quantity", sequence * get_quantity("dimensionless"))
# iterate the sequence and manually convert to base units.
try:
base_unit_sequence = [x.to_base_units() for x in sequence]
except AttributeError:
msg = "Not all values in sequence are quantities."
raise UnitError(msg)
if not len(base_unit_sequence):
return np.array([]) * get_quantity("dimensionless")
return cast("Quantity", np.array([]) * get_quantity("dimensionless"))
units = {x.units for x in base_unit_sequence}
if len(units) != 1:
msg = "Not all values in sequence have compatible units."
Expand Down
7 changes: 5 additions & 2 deletions dascore/utils/deprecate.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import functools
from collections.abc import Callable
from typing import Any, TypeVar
from typing import Any, TypeVar, cast

from typing_extensions import deprecated as dep

Expand Down Expand Up @@ -73,6 +73,9 @@ def wrapper(*args: Any, **kwargs: Any):

# Apply typing-level deprecation *to the wrapper* so editors see it
msg = _build_msg(func)
return dep(msg)(wrapper) # type: ignore[return-value]
# functools.wraps makes the wrapper stand in for func, but the
# (*args, **kwargs) signature cannot express that, so the
# substitution has to be asserted rather than derived.
return cast("F", dep(msg)(wrapper))

return _decorate
2 changes: 1 addition & 1 deletion dascore/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ def unbyte(byte_or_str: bytes) -> str: ...
def unbyte(byte_or_str: _T) -> _T: ...


def unbyte(byte_or_str):
def unbyte(byte_or_str) -> str | _T:
"""
Decode a bytes value, passing anything else through unchanged.

Expand Down
1 change: 1 addition & 0 deletions docs/changelog.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f

- **`to_float` handles pint quantities explicitly.** A time quantity now converts to its duration in seconds (`to_float(2 * dc.units.min) == 120.0`), which previously raised `DimensionalityError`. Every other quantity raises `UnitError` instead of being silently converted: pint's `float()` reduces a *dimensionless* quantity to base units, so a data size came back eight times too large (`25 MB` → `2e8`, the count in bits). Use `convert_units` or `get_byte_count` for an explicit conversion. Relatedly, filtering a coordinate that has no units with *any* quantity — `patch.notch_filter(distance=5 * dc.units.m)`, and dimensionless ones such as `20 %` — now raises `UnitError` with an explanatory message. Previously a dimensionless quantity was silently read as a bare number (`20 %` became 0.2 Hz) and a dimensional one leaked pint's `DimensionalityError`.
- `dascore.utils.time.to_int` and `to_float` are now overloaded wrappers over private `singledispatch` implementations, so a `Series` input is typed as returning a `Series` and an array as returning an array. Their runtime behaviour is unchanged, but `to_int.register(...)` and `to_float.register(...)` no longer exist; register new implementations on `_to_int` / `_to_float` instead. `convert_units` no longer declares a constrained `numeric` type variable — it accepted (and still accepts) `None`, quantities, and numpy scalars, none of which that variable admitted. `WARNING_ACTIONS` no longer lists `"all"`, which Python only began accepting in 3.14 and which raises on the 3.11–3.13 interpreters DASCore also supports; use `"always"`, which it aliases.
- `BaseCoord.size` is now a builtin `int` rather than an `np.int64`, and `1` rather than `1.0` for a shapeless coord, since it is computed with `math.prod` instead of `np.prod`. `Patch.sobel_filter`, `Patch.notch_filter` and `velocity_to_strain_rate_edgeless` build their result with `patch.new(...)` instead of a bare `dc.Patch(...)`, so a `Patch` subclass now survives them as their signatures already promised. Several other return annotations were corrected to describe long-standing behaviour rather than change it: `CoordManager.drop_disassociated_coords` and `CoordManager.drop_private_coords` return a `(coord_manager, array)` tuple, `BaseCoord.get_next_index` returns an array for a sized value and a numpy integer otherwise, `BaseCoord.unit_str` is `None` for a coord carrying no units, and `dc.write` returns the path it was handed, which is not always a `Path`.
- `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.
Expand Down
8 changes: 3 additions & 5 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,9 @@ line-ending = "lf"
[tool.ty.src]
include = ["dascore"] # tests/ still has many diagnostics; expand scope later.

# Rules with large pre-existing error counts, ignored until incrementally
# burned down. Count as of 2026-08-07: invalid-return-type 26.
# invalid-method-override and invalid-argument-type reached zero and are on.
[tool.ty.rules]
invalid-return-type = "ignore"
# No rule is ignored any more: invalid-method-override,
# invalid-argument-type and invalid-return-type have each been burned down
# to zero and left on. Widening [tool.ty.src] to tests/ is the next step.

# These files lazily import optional or untyped modules (xarray, numba,
# h5py.h5r) that are not installed in the pre-commit hook's environment.
Expand Down
20 changes: 19 additions & 1 deletion tests/test_io/test_dasvader/test_dasvader.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
DependencyError,
UnknownFiberFormatError,
)
from dascore.io.dasvader.utils import _dereference, _julia_ms_to_datetime64
from dascore.io.dasvader.utils import (
EXPECTED,
_dereference,
_is_dasvader_jld2,
_julia_ms_to_datetime64,
)
from dascore.utils.downloader import fetch


Expand Down Expand Up @@ -324,3 +329,16 @@ def __getitem__(self, value):
match = r"legacy\.jld2.*'htime'.*h5py<3\.16"
with pytest.raises(DASVaderCompatibilityError, match=match):
_dereference(BrokenResource(), Reference(), "htime")

def test_missing_data_name_returns_false(self):
"""A file with no recognized data name is rejected with a real bool."""

class _Resource:
"""Minimal resource exposing only the non-data field names."""

def get(self, name):
return np.zeros(1, dtype=[(x, "<f8") for x in EXPECTED])

# `is False` rather than a truthiness check: the empty intersection
# used to be returned directly, so the answer was the empty set.
assert _is_dasvader_jld2(_Resource()) is False
Loading