From e275bb166819da7de5b2b7c7623232a921e801e3 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 1 Aug 2026 15:12:37 +0200 Subject: [PATCH 1/8] Enable ty's unresolved-attribute rule outside coords/spool Take unresolved-attribute from 138 diagnostics to 26 and enforce the rule everywhere except core/coords.py and core/spool.py, which are held back by a documented temporary override until the coord/spool API questions behind them are settled. Most fixes are hints that were wrong: optional_import is overloaded so its default raise-on-missing mode returns a module; the h5 caster classes inherit the managed handle under TYPE_CHECKING (FiberIO methods annotate the caster but receive the handle); the index reads its frames by column instead of through per-row namedtuples; the xml_binary helpers take XMLBinaryInfo, not XMLLaserZones; to_summary_dict only ever returns CoordSummary. The dynamic markers stamped by the type-caster and patch_function decorators are now declared as Protocols. Three latent bugs surfaced along the way: a NaT timedelta bound crashed _coord_record_from_row, maybe_mem_map leaned on AttributeError from fid.name, and the binary-ufunc error path raised AttributeError instead of UnitError for unitless operands. --- dascore/core/coordmanager.py | 34 +++++----- dascore/core/coords.py | 12 +++- dascore/io/core.py | 53 +++++++++++---- dascore/io/index/backend.py | 43 ++++++------ dascore/io/index/catalog.py | 26 ++++---- dascore/io/index/indexer.py | 12 ++-- dascore/io/index/ingest.py | 32 ++++++--- dascore/io/index/planned.py | 3 +- dascore/io/index/query.py | 5 +- dascore/io/index/schema.py | 87 +++++++++++++++++++++++++ dascore/io/tdms/utils.py | 4 +- dascore/io/xml_binary/utils.py | 6 +- dascore/proc/coords.py | 1 + dascore/units.py | 6 +- dascore/utils/array.py | 3 +- dascore/utils/hdf5.py | 12 +++- dascore/utils/jit.py | 5 +- dascore/utils/misc.py | 31 +++++++-- dascore/utils/patch.py | 30 +++++++-- dascore/utils/patch_assembly.py | 1 + dascore/utils/pd.py | 21 +++++- pyproject.toml | 17 +++-- tests/test_io/test_index/test_schema.py | 11 +++- 23 files changed, 351 insertions(+), 104 deletions(-) diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py index a3de3797c..5269895ad 100644 --- a/dascore/core/coordmanager.py +++ b/dascore/core/coordmanager.py @@ -123,7 +123,7 @@ def _get_indexers_and_new_coords_dict( ): """Get reductions for each dimension.""" dim_reductions = {x: slice(None, None) for x in cm.dims} - new_coords = dict(cm._get_dim_array_dict(keep_coord=True)) + new_coords = dict(cm._get_dim_coord_dict()) for coord_name, vals in kwargs.items(): # All coordinates should exist in coord_map (filtered by # _get_single_dim_kwarg_list) @@ -223,7 +223,9 @@ def __getitem__(self, item) -> BaseCoord: def __getattr__(self, item) -> BaseCoord: try: - return super().__getattr__(item) + # pydantic defines BaseModel.__getattr__ only at runtime so + # checkers still flag misspelled fields. + return super().__getattr__(item) # ty: ignore[unresolved-attribute] except AttributeError: # unlike get item, get attr returns the base coordinate. try: @@ -295,7 +297,7 @@ def _divide_kwargs(kwargs): indirect_coord_drops = _get_dim_change_drop(coord_map, dim_map) # drop coords then call get_coords to handle adding new ones. coords, _ = self.drop_coords(*(coord_to_drop + indirect_coord_drops)) - out = coords._get_dim_array_dict(keep_coord=True) + out = coords._get_dim_coord_dict() out.update({i: v for i, v in kwargs.items() if i not in coord_to_drop}) # update based on keywords for item, value in coord_updates.items(): @@ -800,20 +802,18 @@ def validate_data(self, data): raise CoordDataError(msg) return data - def _get_dim_array_dict( - self, keep_coord=False - ) -> dict[str, tuple[tuple[str, ...], ArrayLike | BaseCoord]]: - """ - Get the coord map in the form: - {coord_name = ((dims,), array)}. + def _get_dim_coord_dict(self) -> dict[str, tuple[tuple[str, ...], BaseCoord]]: + """Get the coord map in the form {coord_name: ((dims,), coord)}.""" + return { + name: (self.dim_map[name], coord) for name, coord in self.coord_map.items() + } - if keep_coord, just keep the coordinate as second arg. - """ - out = {} - for name, coord in self.coord_map.items(): - dims = self.dim_map[name] - out[name] = (dims, coord if keep_coord else coord.data) - return out + def _get_dim_array_dict(self) -> dict[str, tuple[tuple[str, ...], ArrayLike]]: + """Get the coord map in the form {coord_name: ((dims,), array)}.""" + return { + name: (dims, coord.data) + for name, (dims, coord) in self._get_dim_coord_dict().items() + } def set_units(self, **kwargs): """Set the units of the coordinate manager.""" @@ -1015,7 +1015,7 @@ def keys(self): """Return the keys (coordinates) in the coord manager.""" return self.coord_map.keys() - def to_summary_dict(self) -> dict[str, CoordSummary | tuple[str, ...]]: + def to_summary_dict(self) -> dict[str, CoordSummary]: """Convert the contents of the coordinate manager to a summary dict.""" dim_map = self.dim_map out = {} diff --git a/dascore/core/coords.py b/dascore/core/coords.py index e34d98e79..16042d93f 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -18,7 +18,7 @@ from functools import cache from operator import gt, lt from types import EllipsisType -from typing import Any, Literal, overload +from typing import TYPE_CHECKING, Any, Literal, overload import numpy as np import pandas as pd @@ -291,6 +291,16 @@ class BaseCoord(DascoreBaseModel, abc.ABC): shape: tuple[int, ...] | None = None dtype: Any = None + if TYPE_CHECKING: + # Every coord exposes its values, but the array-backed coords store + # them in a pydantic field while the rest compute them in a property. + # Pydantic refuses to let a field shadow an inherited property (and a + # field here would make values a required init argument), so the + # shared interface is only declared for type checkers. + @property + def values(self) -> ArrayLike: + """The coordinate's values.""" + _rich_style = dascore_styles["default_coord"] _evenly_sampled = False _sorted = False diff --git a/dascore/io/core.py b/dascore/io/core.py index 65a910230..45d90d634 100644 --- a/dascore/io/core.py +++ b/dascore/io/core.py @@ -8,12 +8,20 @@ import inspect import warnings from collections import defaultdict -from collections.abc import Generator, Mapping +from collections.abc import Callable, Generator, Mapping from functools import cached_property, wraps from numbers import Integral from pathlib import Path from threading import RLock -from typing import Any, Literal, NotRequired, TypedDict, get_type_hints +from typing import ( + Any, + Literal, + NotRequired, + Protocol, + TypedDict, + cast, + get_type_hints, +) import numpy as np import pandas as pd @@ -713,7 +721,7 @@ def _get_format( # may happen in each fiber_ios get_format method, many of which # may be third party code. func = fiber_io.get_format - required_type = func._required_type + required_type = _required_resource_type(func) func_input = None try: # Get resource has to be in the try block because it can also @@ -752,6 +760,27 @@ def _get_input_type_name(self, obj): # ------------- Protocol for File Format support +class _TypeCasterMethod(Protocol): + """ + A FiberIO method wrapped by the type caster. + + The caster stamps these markers onto the wrapped method so the io + machinery can find the original function and the resource type the + method wants its input coerced to. + """ + + func: Callable + _type_caster_wrapped: bool + _required_type: type | None + + def __call__(self, *args, **kwargs): ... + + +def _required_resource_type(method) -> type | None: + """Return the resource type a FiberIO method's caster coerces its input to.""" + return cast(_TypeCasterMethod, method)._required_type + + def _type_caster(func, sig, required_type, arg_name): """A decorator for casting types for arguments of cast ind.""" fun_name = func.__name__ @@ -791,14 +820,15 @@ def _wrapper(*args, _pre_cast=False, **kwargs): return out # attach the function and required type for later use - _wrapper.func = func + caster = cast(_TypeCasterMethod, _wrapper) + caster.func = func # subclasses of FIBERIO subclasses can wrap this twice, so we mark # it to avoid that scenario. - _wrapper._type_caster_wrapped = True + caster._type_caster_wrapped = True # also specify required type - _wrapper._required_type = required_type + caster._required_type = required_type - return _wrapper + return caster def _is_wrapped_func(func1, func2): @@ -979,8 +1009,9 @@ def __init_subclass__(cls, **kwargs): msg = "You must specify the file format with the name field." raise InvalidFiberIOError(msg) # register fiber_io - manager: _FiberIOManager = cls.__mro__[1].manager - manager.register_fiberio(cls()) + parent = cls.__mro__[1] + assert issubclass(parent, FiberIO) # only FiberIO subclasses get here + parent.manager.register_fiberio(cls()) # decorate methods for type-casting for name, param_ind in cls._automatic_type_casters.items(): method = getattr(cls, name) @@ -1059,7 +1090,7 @@ def read( fiber_io = FiberIO.manager.get_fiberio( format=file_format, version=file_version ) - required_type = fiber_io.read._required_type + required_type = _required_resource_type(fiber_io.read) path = man.get_resource(required_type) out = fiber_io.read( path, @@ -1667,7 +1698,7 @@ def write( patch_or_spool = _maybe_split_gapped_patches(patch_or_spool, fiber_io, split) with IOResourceManager(path) as man: func = fiber_io.write - required_type = func._required_type + required_type = _required_resource_type(func) resource = man.get_resource(required_type) func(patch_or_spool, resource, _pre_cast=True, **kwargs) return path diff --git a/dascore/io/index/backend.py b/dascore/io/index/backend.py index 9745e5752..c8643b671 100644 --- a/dascore/io/index/backend.py +++ b/dascore/io/index/backend.py @@ -306,15 +306,11 @@ def _ensure_attr_columns_for( failing the whole index update. """ meta = self._attr_meta() - mapping = { - (row.attr_name, row.value_kind): row.column_name - for row in meta.itertuples() - } + keys = list(zip(meta["attr_name"], meta["value_kind"], strict=True)) + mapping = dict(zip(keys, meta["column_name"], strict=True)) stored_units = { - (row.attr_name, row.value_kind): ( - None if pd.isnull(row.units) else row.units - ) - for row in meta.itertuples() + key: (None if pd.isnull(units) else units) + for key, units in zip(keys, meta["units"], strict=True) } taken = set(mapping.values()) observed: dict[tuple[str, str], set[str | None]] = {} @@ -563,9 +559,9 @@ def _existing_ordinals(self, by_base: dict[str, list[str]]) -> dict: f"WHERE source_path IN ({marks}) AND base_uri = ?", [*chunk, base_uri], ) - for row in df.itertuples(): - if not pd.isnull(row.ordinal): - out[(base_uri, row.source_path)] = int(row.ordinal) + for path, ordinal in zip(df["source_path"], df["ordinal"], strict=True): + if not pd.isnull(ordinal): + out[(base_uri, path)] = int(ordinal) return out def renumber_ordinals_by_time(self) -> None: @@ -647,10 +643,11 @@ def move_sources( # attr name -> [(kind, column)] once; per-move dataframe # filtering dominated large renames. kinds_by_name: dict[str, list[tuple[str, str]]] = {} - for row in self._attr_meta().itertuples(): - kinds_by_name.setdefault(row.attr_name, []).append( - (row.value_kind, row.column_name) - ) + meta = self._attr_meta() + for name, kind, column in zip( + meta["attr_name"], meta["value_kind"], meta["column_name"], strict=True + ): + kinds_by_name.setdefault(name, []).append((kind, column)) now = time.time_ns() ids: dict[str, int] = {} for chunk, marks in self._iter_in_batches(list(moves)): @@ -866,22 +863,24 @@ def _flatten( kinds = set(rows["value_kind"]) multi_kind = len(rows) > 1 series = None - for row in rows.itertuples(): - if row.column_name not in out: + for column, kind in zip( + rows["column_name"], rows["value_kind"], strict=True + ): + if column not in out: continue - col = out[row.column_name] - if row.value_kind == "time": + col = out[column] + if kind == "time": col = _ns_to_time(col, "datetime") - elif row.value_kind == "dur": + elif kind == "dur": col = _ns_to_time(col, "timedelta") - elif row.value_kind == "bool": + elif kind == "bool": col = col.astype("boolean") if multi_kind: # multi-kind attrs coalesce in object space; typed # extension arrays refuse cross-dtype fills col = col.astype(object).where(col.notna(), np.nan) series = col if series is None else series.where(series.notna(), col) - cols_to_drop.append(row.column_name) + cols_to_drop.append(column) if series is not None: if kinds == {"str"}: # flat-contract convention: missing strings are "" diff --git a/dascore/io/index/catalog.py b/dascore/io/index/catalog.py index 9a658bd9a..f5ade832f 100644 --- a/dascore/io/index/catalog.py +++ b/dascore/io/index/catalog.py @@ -91,11 +91,12 @@ def for_patch_coord(self, coord) -> tuple: return self.magnitudes # a unit-bearing query keeps its own dimensionality; a bare # numeric query means canonical SI in the coord's dimension - base = ( - get_quantity(self.units) - if self.units is not None - else get_quantity(str(coord_units)).to_base_units().units - ) + if self.units is not None: + base = get_quantity(self.units) + else: + coord_quant = get_quantity(str(coord_units)) + assert coord_quant is not None # the coord has units in this branch + base = coord_quant.to_base_units().units return tuple(None if mag is None else mag * base for mag in self.magnitudes) @@ -200,7 +201,7 @@ def resolve(self, row: Mapping, **trim) -> dc.Patch: re-applied above, so ignoring them is slower, never wrong. """ - def live_entries(self) -> Mapping[str, dc.Patch]: + def live_entries(self) -> dict[str, dc.Patch]: """Return the live patches this resolver serves (path -> patch).""" return {} @@ -220,7 +221,7 @@ def __init__(self, patches: Sequence[dc.Patch] = ()): _patch_path(patch): patch for patch in patches } - def live_entries(self) -> Mapping[str, dc.Patch]: + def live_entries(self) -> dict[str, dc.Patch]: """Return the live patch registry.""" return self._registry @@ -308,7 +309,7 @@ def __init__(self): # plan:/// prefix -> the PlanResolver that owns it self.plans: dict[str, PatchResolver] = {} - def live_entries(self) -> Mapping[str, dc.Patch]: + def live_entries(self) -> dict[str, dc.Patch]: """Return the merged live patch registry.""" return self.live._registry @@ -682,12 +683,13 @@ def __getstate__(self) -> dict: # patch, not N — the payload Spool.map ships per task) — in # presentation order, so a rebuilt registry keeps the view's # ordering. - if self.is_view and self.resolver.live_entries(): + resolver = self.resolver + if self.is_view and resolver is not None and resolver.live_entries(): df = self.to_df() paths = list(dict.fromkeys(df["path"].astype(str))) - entries = self.resolver.live_entries() + entries = resolver.live_entries() keep = {k: entries[k] for k in paths if k in entries} - state["resolver"] = _membership_resolver(self.resolver, keep, paths) + state["resolver"] = _membership_resolver(resolver, keep, paths) return state def _view(self, queries, residuals, order=_KEEP, ids=_KEEP) -> PatchCatalog: @@ -969,6 +971,7 @@ def resolve_row(self, row: Mapping, extra_trim: Mapping | None = None) -> dc.Pat } ) trim_hint.update(extra_trim or {}) + assert self.resolver is not None # rows only exist once one is set patch = self.resolver.resolve(row, **trim_hint) return apply_exact_residuals(patch, self._residuals) @@ -987,6 +990,7 @@ def __iter__(self): if live is not None: yield from live return + assert df is not None # the frame is fetched when there are no live values for index in range(len(df)): try: yield self.resolve_row(df.iloc[index].to_dict()) diff --git a/dascore/io/index/indexer.py b/dascore/io/index/indexer.py index b8364580d..3d7be4f9e 100644 --- a/dascore/io/index/indexer.py +++ b/dascore/io/index/indexer.py @@ -356,13 +356,15 @@ def update(self, paths=None, progress: PROGRESS_LEVELS = "standard") -> Self: stats and rewritten in place, never rescanned (see _detect_moves). """ files = self._walk() + stats = self._backend.source_stats() stored = { - row.source_path: ( - None - if pd.isnull(row.mtime_ns) - else (int(row.mtime_ns), int(row.size_bytes)) + path: (None if pd.isnull(mtime) else (int(mtime), int(size))) + for path, mtime, size in zip( + stats["source_path"], + stats["mtime_ns"], + stats["size_bytes"], + strict=True, ) - for row in self._backend.source_stats().itertuples() } stale = [path for path in stored if path not in files] changed = [ diff --git a/dascore/io/index/ingest.py b/dascore/io/index/ingest.py index cb25d004b..cea938eed 100644 --- a/dascore/io/index/ingest.py +++ b/dascore/io/index/ingest.py @@ -19,9 +19,17 @@ import pandas as pd from dascore.core.summary import PatchSummary, normalize_source_patch_id -from dascore.io.index.schema import KINDS, RESERVED_ATTR_COLUMNS +from dascore.io.index.schema import ( + KINDS, + RESERVED_ATTR_COLUMNS, + CoordDefRow, + PatchCoordRow, + PatchRow, + SourceRow, +) from dascore.units import get_quantity from dascore.utils.paths import parse_hive_path_attrs +from dascore.utils.pd import iter_rows from dascore.utils.time import to_datetime64, to_int, to_timedelta64 _SANITIZE_RE = re.compile(r"[^a-z0-9_]+") @@ -148,7 +156,9 @@ def attr_column_name(name: str, kind: str) -> str: def _base_unit_info(value, unit_str: str | None = None) -> tuple[float, str]: """Return a value's base-unit magnitude and canonical unit string.""" quant = value if unit_str is None else value * get_quantity(unit_str) - quant = get_quantity(quant).to_base_units() + quant = get_quantity(quant) + assert quant is not None # unit-bearing values only + quant = quant.to_base_units() return float(quant.magnitude), str(quant.units) @@ -505,10 +515,16 @@ def assemble_source_records( if "patch_id" in patches.columns: patches = patches.sort_values("patch_id") col_info = { - row.column_name: (row.attr_name, row.value_kind, _py_scalar(row.units)) - for row in meta.itertuples() + column: (name, kind, _py_scalar(units)) + for column, name, kind, units in zip( + meta["column_name"], + meta["attr_name"], + meta["value_kind"], + meta["units"], + strict=True, + ) } - def_map = {int(row.coord_def_id): row for row in defs.itertuples()} + def_map = {int(row.coord_def_id): row for row in iter_rows(defs, CoordDefRow)} attr_rows = ( {int(k): v for k, v in attrs.set_index("patch_id").to_dict("index").items()} if not attrs.empty @@ -523,12 +539,12 @@ def assemble_source_records( else {} ) out = [] - for src in sources.itertuples(): + for src in iter_rows(sources, SourceRow): sub = patches_by_source.get(int(src.source_id)) if sub is None: continue patch_records = [] - for patch in sub.itertuples(): + for patch in iter_rows(sub, PatchRow): pid = int(patch.patch_id) typed = {} for col, value in attr_rows.get(pid, {}).items(): @@ -538,7 +554,7 @@ def assemble_source_records( kind=kind, value=_py_scalar(value), units=units ) coords = [] - for link in link_groups.get(pid, pd.DataFrame()).itertuples(): + for link in iter_rows(link_groups.get(pid, pd.DataFrame()), PatchCoordRow): cdef = def_map[int(link.coord_def_id)] coords.append( CoordRecord( diff --git a/dascore/io/index/planned.py b/dascore/io/index/planned.py index d9148c0e9..a4c597534 100644 --- a/dascore/io/index/planned.py +++ b/dascore/io/index/planned.py @@ -104,7 +104,8 @@ def _coord_record_from_row( lo, hi = pd.Timestamp(lo).to_datetime64(), pd.Timestamp(hi).to_datetime64() dtype = "datetime64[ns]" elif isinstance(lo, pd.Timedelta | np.timedelta64): - lo, hi = pd.Timedelta(lo).to_timedelta64(), pd.Timedelta(hi).to_timedelta64() + # dascore's converter (unlike Timedelta.to_timedelta64) handles NaT. + lo, hi = dc.to_timedelta64(lo), dc.to_timedelta64(hi) dtype = "timedelta64[ns]" else: lo, hi = float(lo), float(hi) diff --git a/dascore/io/index/query.py b/dascore/io/index/query.py index d0004568b..75ccc9d58 100644 --- a/dascore/io/index/query.py +++ b/dascore/io/index/query.py @@ -216,7 +216,10 @@ def build_attr_clause( raise InvalidSpoolQueryError(msg) kinds = set(rows["value_kind"]) columns = dict(zip(rows["value_kind"], rows["column_name"])) - units = {row.value_kind: _normalize_unit(row.units) for row in rows.itertuples()} + units = { + kind: _normalize_unit(unit) + for kind, unit in zip(rows["value_kind"], rows["units"], strict=True) + } def col(kind): return f"a.{dialect.quote(columns[kind])}" diff --git a/dascore/io/index/schema.py b/dascore/io/index/schema.py index 8aee1483d..e6fe634d6 100644 --- a/dascore/io/index/schema.py +++ b/dascore/io/index/schema.py @@ -9,6 +9,7 @@ from __future__ import annotations from types import MappingProxyType +from typing import NamedTuple # Version of the index schema, independent of dascore's version. INDEX_VERSION = 4 @@ -242,6 +243,92 @@ # non-private columns. SPOOL_HIDDEN_COLUMNS = ("n_dims", "sample_count_total", "shape") +# --- Row views ------------------------------------------------------- +# +# Index code reads table rows with `iter_rows(df, Row)` (see +# dascore.utils.pd), which names the row shape pandas builds dynamically +# in `itertuples`. Fields mirror the table definitions above (a test keeps +# them in step) and nullable columns are typed with None, though pandas +# may surface them as NaN — reading code guards with `pd.isnull`. A frame +# holding only some of a table's columns still uses its table's row view; +# only the columns actually fetched can be read. + + +class SourceRow(NamedTuple): + """A row of the sources table.""" + + source_id: int + base_uri: str + source_path: str + source_format: str + format_version: str + mtime_ns: int | None + size_bytes: int | None + path_attrs: str | None + last_indexed_ns: int + ordinal: int + + +class PatchRow(NamedTuple): + """A row of the patches table.""" + + patch_id: int + source_id: int + source_patch_id: str + n_dims: int + dims: str + shape: str + sample_count_total: int + time_min: int | None + time_max: int | None + time_step: int | None + distance_min: float | None + distance_max: float | None + distance_step: float | None + + +class CoordDefRow(NamedTuple): + """A row of the coord_defs table.""" + + coord_def_id: int + def_key: str + fingerprint: str | None + value_kind: str + dtype: str + length: int + units: str | None + min_num: float | None + max_num: float | None + step_num: float | None + min_ns: int | None + max_ns: int | None + step_ns: int | None + min_str: str | None + max_str: str | None + is_monotonic: bool | None + is_relative: bool | None + + +class PatchCoordRow(NamedTuple): + """A row of the patch_coords table.""" + + patch_id: int + coord_name: str + coord_dims: str + coord_def_id: int + + +# Row view for each stored table, used to check the views stay in step +# with the column definitions. +TABLE_ROWS = MappingProxyType( + { + "sources": SourceRow, + "patches": PatchRow, + "coord_defs": CoordDefRow, + "patch_coords": PatchCoordRow, + } +) + # Explicit secondary indexes. Every other access path is covered by a # PRIMARY KEY or UNIQUE autoindex above — patch_coords(patch_id, # coord_name), sources(base_uri, source_path), patches(source_id, diff --git a/dascore/io/tdms/utils.py b/dascore/io/tdms/utils.py index 85e0f3415..9e94193b8 100644 --- a/dascore/io/tdms/utils.py +++ b/dascore/io/tdms/utils.py @@ -5,6 +5,7 @@ import datetime import mmap import struct +from collections.abc import Callable from typing import Any import numpy as np @@ -49,7 +50,8 @@ def type_not_supported(vargin): ) # Function mapping for reading TDMS data types -TDS_READ_VAL = dict( +# Values differ by type, so the readers are only typed as callables. +TDS_READ_VAL: dict[str, Callable] = dict( { "void": lambda f: None, # tdsTypeVoid "int8": lambda f: struct.unpack(" None: raise UnitError(msg) from e -def get_inverted_quant(quant, data_units): +def get_inverted_quant(quant: Quantity | None, data_units): """Convert to inverted units.""" if quant is None: return quant, True @@ -382,8 +382,8 @@ def _check_to_units(to_unit, dim): _check_to_units(to_unit, dim) # get inverse of desired output units and ensure units are pure. to_quant = get_quantity(to_unit) - assert to_quant.magnitude == 1.0 - to_units = get_quantity(to_unit).units + assert to_quant is not None and to_quant.magnitude == 1.0 + to_units = to_quant.units quant1, quant2 = get_quantity(arg1), get_quantity(arg2) _ensure_same_units(quant1, quant2) out1, inverted1 = get_inverted_quant(quant1, to_units) diff --git a/dascore/utils/array.py b/dascore/utils/array.py index 2314b3aeb..4da6b45cc 100644 --- a/dascore/utils/array.py +++ b/dascore/utils/array.py @@ -256,7 +256,8 @@ def _apply_op_units(patch, other, operator, attrs, reversed=False): try: new_data_w_units = _apply_op(data, other, operator, reversed=reversed) except DimensionalityError as er: - msg = f"{operator} failed with units {data_units} and {other.units}" + other_units = getattr(other, "units", None) + msg = f"{operator} failed with units {data_units} and {other_units}" raise UnitError(msg) from er # Check if result has units (comparison operators return plain arrays) if hasattr(new_data_w_units, "units"): diff --git a/dascore/utils/hdf5.py b/dascore/utils/hdf5.py index 84602cc73..83f8f11cf 100644 --- a/dascore/utils/hdf5.py +++ b/dascore/utils/hdf5.py @@ -10,6 +10,7 @@ from contextlib import suppress from functools import partial from pathlib import Path +from typing import TYPE_CHECKING import numpy as np import pandas as pd @@ -181,7 +182,16 @@ def open_h5_resource( raise NotImplementedError(msg) -class H5Reader: +# FiberIO read/scan/get_format annotate the *caster* class (H5Reader and +# friends): the io machinery swaps the annotated resource for whatever +# `get_handle` returns, so what those methods actually receive is the +# managed handle. Inheriting it while type checking makes the annotation +# describe the value the method really gets; nothing is instantiated at +# runtime, where the casters stay plain classes. +_H5CasterBase = _ManagedH5pyFile if TYPE_CHECKING else object + + +class H5Reader(_H5CasterBase): """A thin wrapper around h5py for reading files. Remote UPath resources stay remote-first and transparently retry against diff --git a/dascore/utils/jit.py b/dascore/utils/jit.py index dd8bef580..90525decd 100644 --- a/dascore/utils/jit.py +++ b/dascore/utils/jit.py @@ -101,7 +101,10 @@ def decorated(*args, **kwargs): out_func = decorated else: - out_func = numba.jit(**compiler_kwargs)(func) + # numba is the real module in this branch; the dummy stands in + # only when the import failed. + jit = numba.jit # ty: ignore[unresolved-attribute] + out_func = jit(**compiler_kwargs)(func) # Make the original func accessible via .func; function objects accept # new attributes even though their declared type does not. out_func.func = func # ty: ignore[invalid-assignment] diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index b8a474380..7a55ebc9b 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -15,7 +15,7 @@ from io import IOBase from pathlib import Path from types import ModuleType -from typing import Literal +from typing import Literal, overload import numpy as np import pandas as pd @@ -432,6 +432,22 @@ def iterate(obj): return obj if isinstance(obj, Iterable) else (obj,) +@overload +def optional_import( + package_name: str, + on_missing: Literal["raise"] = "raise", + required_for: str = "the requested functionality", +) -> ModuleType: ... + + +@overload +def optional_import( + package_name: str, + on_missing: Literal["warn", "ignore"], + required_for: str = "the requested functionality", +) -> ModuleType | None: ... + + def optional_import( package_name: str, on_missing: Literal["raise", "warn", "ignore"] = "raise", @@ -557,10 +573,12 @@ def get_stencil_coefs(order, derivative=2): def get_parent_code_name(levels: int = 2) -> str: """Get the name of the calling function/class levels up in stack.""" - stack = inspect.currentframe() + frame = inspect.currentframe() for _ in range(levels): - stack = stack.f_back - return stack.f_code.co_name + frame = frame.f_back if frame is not None else None + # frames only run out past the top of the stack, well above any caller. + assert frame is not None + return frame.f_code.co_name def to_str(val): @@ -992,8 +1010,11 @@ def maybe_mem_map(fid: IOBase, dtype=" np.ndarray | np.memmap: fid A buffered reader, e.g. from open(file) as fid. """ + # File objects backed by memory (BytesIO and friends) have no name; + # they fall through to the in-memory read below. + name = getattr(fid, "name", None) try: - raw = np.memmap(fid.name, dtype=dtype, mode="r") + raw = np.memmap(name, dtype=dtype, mode="r") except (AttributeError, TypeError, ValueError): # Fallback: read into memory fid.seek(0) diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index a886dc95c..63abbe438 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 +from typing import Any, Literal, Protocol, cast import numpy as np import pandas as pd @@ -94,7 +94,9 @@ def _func_and_kwargs_str(func: Callable, patch, *args, **kwargs) -> str: f"{k}={_format_values(v)!r}" for k, v in kwargs_.items() if v is not None ] arguments.sort() - out = f"{func.__name__}(" + # partials and other callables without a name still get a history entry. + name = getattr(func, "__name__", str(func)) + out = f"{name}(" if arguments: out += f"{','.join(arguments)}" return out + ")" @@ -196,6 +198,21 @@ def check_patch_attrs(patch: PatchType, required_attrs: attr_type) -> PatchType: return patch +class _PatchFunction(Protocol): + """ + A function wrapped by `patch_function`. + + The decorator attaches references back to the function it wrapped so + callers can skip the patch-function machinery when calling it again. + """ + + func: Callable + raw_function: Callable + __wrapped__: Callable + + def __call__(self, patch, *args, **kwargs): ... + + def patch_function( required_dims: tuple[str, ...] | Callable | None = None, required_coords: tuple[str, ...] | None = None, @@ -311,12 +328,13 @@ def _func(patch, *args, **kwargs): # Attach original function. Although we want to encourage raw_function # for consistency with pydantic, we leave this to not break old code. - _func.func = getattr(func, "raw_function", func) + patch_func = cast(_PatchFunction, _func) + patch_func.func = getattr(func, "raw_function", func) # matches pydantic naming. - _func.raw_function = getattr(func, "raw_function", func) - _func.__wrapped__ = func + patch_func.raw_function = getattr(func, "raw_function", func) + patch_func.__wrapped__ = func - return _func + return patch_func if callable(required_dims): # the decorator is used without parens return patch_function()(required_dims) diff --git a/dascore/utils/patch_assembly.py b/dascore/utils/patch_assembly.py index 3b94d0a07..643e66707 100644 --- a/dascore/utils/patch_assembly.py +++ b/dascore/utils/patch_assembly.py @@ -224,6 +224,7 @@ 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 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/dascore/utils/pd.py b/dascore/utils/pd.py index 239a93b88..e5aad1506 100644 --- a/dascore/utils/pd.py +++ b/dascore/utils/pd.py @@ -4,8 +4,9 @@ import fnmatch from collections import defaultdict -from collections.abc import Collection, Generator, Mapping, Sequence +from collections.abc import Collection, Generator, Iterator, Mapping, Sequence from functools import cache +from typing import TypeVar, cast import numpy as np import pandas as pd @@ -18,6 +19,24 @@ from dascore.utils.misc import is_range, order_range_tuple, sanitize_range_param from dascore.utils.time import to_datetime64, to_timedelta64 +_RowType = TypeVar("_RowType") + + +def iter_rows(df: pd.DataFrame, row_type: type[_RowType]) -> Iterator[_RowType]: + """ + Iterate over a dataframe's rows as named tuples of a known shape. + + Parameters + ---------- + df + The dataframe to iterate. + row_type + A NamedTuple declaring the columns the caller reads. Pandas builds + the row tuple dynamically, so this only names the shape for + readers (and type checkers); it is never instantiated. + """ + return cast(Iterator[_RowType], df.itertuples()) + @cache def get_regex(seed_str): diff --git a/pyproject.toml b/pyproject.toml index ee3344715..a19de70b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -260,12 +260,11 @@ line-ending = "lf" include = ["dascore"] # tests/ still has many diagnostics; expand scope later. # Rules with large pre-existing error counts, ignored until incrementally -# burned down. Counts as of 2026-08-01: invalid-argument-type 179, -# unresolved-attribute 138, invalid-return-type 72, invalid-method-override 35, -# not-subscriptable 19, no-matching-overload 12. +# burned down. Counts as of 2026-08-01: invalid-argument-type 175, +# invalid-return-type 72, invalid-method-override 36, no-matching-overload 13, +# not-subscriptable 5. [tool.ty.rules] invalid-argument-type = "ignore" -unresolved-attribute = "ignore" invalid-return-type = "ignore" no-matching-overload = "ignore" not-subscriptable = "ignore" @@ -279,6 +278,16 @@ include = ["dascore/compat.py", "dascore/utils/jit.py", "dascore/io/dasvader/uti [tool.ty.overrides.rules] unresolved-import = "ignore" +# The last 26 unresolved-attribute errors are two API questions rather than +# wrong hints: which members BaseCoord promises (start/stop/_get_index) and +# whether a PatchCatalog can exist without a backing catalog/resolver. Both +# are settled, with the rest of the coord/spool API, in a follow-up PR. +[[tool.ty.overrides]] +include = ["dascore/core/coords.py", "dascore/core/spool.py"] + +[tool.ty.overrides.rules] +unresolved-attribute = "ignore" + [tool.typos.files] extend-exclude = ["docs/_static/logo.svg"] diff --git a/tests/test_io/test_index/test_schema.py b/tests/test_io/test_index/test_schema.py index eac332b7f..556b68b34 100644 --- a/tests/test_io/test_index/test_schema.py +++ b/tests/test_io/test_index/test_schema.py @@ -10,7 +10,16 @@ from dascore.exceptions import InvalidIndexError, InvalidIndexVersionError from dascore.io.index import get_backend -from dascore.io.index.schema import INDEX_VERSION, TABLES +from dascore.io.index.schema import INDEX_VERSION, TABLE_ROWS, TABLES + + +class TestRowViews: + """The row views index code reads must match the stored columns.""" + + @pytest.mark.parametrize("table", sorted(TABLE_ROWS)) + def test_fields_match_columns(self, table): + """Each row view declares exactly its table's columns.""" + assert TABLE_ROWS[table]._fields == tuple(TABLES[table]) class TestSchemaValidation: From df75d89dfad8626d64bc723b9d9781b9285edf8d Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 1 Aug 2026 15:28:43 +0200 Subject: [PATCH 2/8] Address review: keep name lookup guarded, drop the index from row views The memmap name lookup moves back inside the try so a file object whose name property raises still falls back to the in-memory read, iter_rows passes index=False so the declared row shape lines up positionally with what pandas yields, and a stack shallower than the requested level now returns "" instead of asserting (the helper runs while building error messages, where crashing would mask the real error). --- dascore/io/index/catalog.py | 7 ++++++- dascore/utils/misc.py | 11 +++++------ dascore/utils/pd.py | 5 +++-- tests/test_utils/test_misc.py | 17 +++++++++++++++++ 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/dascore/io/index/catalog.py b/dascore/io/index/catalog.py index f5ade832f..8f756ac81 100644 --- a/dascore/io/index/catalog.py +++ b/dascore/io/index/catalog.py @@ -202,7 +202,12 @@ def resolve(self, row: Mapping, **trim) -> dc.Patch: """ def live_entries(self) -> dict[str, dc.Patch]: - """Return the live patches this resolver serves (path -> patch).""" + """ + Return the live patches this resolver serves (path -> patch). + + This is the registry itself, not a copy: dropping an entry from it + is how a removed source's live patch stops being served. + """ return {} diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index 7a55ebc9b..d0f89d9aa 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -576,8 +576,8 @@ def get_parent_code_name(levels: int = 2) -> str: frame = inspect.currentframe() for _ in range(levels): frame = frame.f_back if frame is not None else None - # frames only run out past the top of the stack, well above any caller. - assert frame is not None + if frame is None: # asked for a frame above the top of the stack + return "" return frame.f_code.co_name @@ -1010,11 +1010,10 @@ def maybe_mem_map(fid: IOBase, dtype=" np.ndarray | np.memmap: fid A buffered reader, e.g. from open(file) as fid. """ - # File objects backed by memory (BytesIO and friends) have no name; - # they fall through to the in-memory read below. - name = getattr(fid, "name", None) try: - raw = np.memmap(name, dtype=dtype, mode="r") + # File objects backed by memory (BytesIO and friends) have no + # usable name; those fall through to the in-memory read below. + raw = np.memmap(getattr(fid, "name", None), dtype=dtype, mode="r") except (AttributeError, TypeError, ValueError): # Fallback: read into memory fid.seek(0) diff --git a/dascore/utils/pd.py b/dascore/utils/pd.py index e5aad1506..deab137c2 100644 --- a/dascore/utils/pd.py +++ b/dascore/utils/pd.py @@ -33,9 +33,10 @@ def iter_rows(df: pd.DataFrame, row_type: type[_RowType]) -> Iterator[_RowType]: row_type A NamedTuple declaring the columns the caller reads. Pandas builds the row tuple dynamically, so this only names the shape for - readers (and type checkers); it is never instantiated. + readers (and type checkers); it is never instantiated. The frame's + index is left out so the declared fields line up with the row's. """ - return cast(Iterator[_RowType], df.itertuples()) + return cast(Iterator[_RowType], df.itertuples(index=False)) @cache diff --git a/tests/test_utils/test_misc.py b/tests/test_utils/test_misc.py index 0f011b549..88a72e657 100644 --- a/tests/test_utils/test_misc.py +++ b/tests/test_utils/test_misc.py @@ -25,6 +25,7 @@ deep_equality_check, get_2d_line_intersection, get_buffer_size, + get_parent_code_name, get_stencil_coefs, iterate, maybe_get_items, @@ -691,6 +692,22 @@ def test_parallel_lines(self): assert np.isnan(out).all() +class TestGetParentCodeName: + """Tests for naming the calling scope.""" + + def test_gets_caller(self): + """Level 1 names the calling scope, level 2 the one above it.""" + + def inner(): + return get_parent_code_name(levels=1), get_parent_code_name(levels=2) + + assert inner() == ("inner", "test_gets_caller") + + def test_above_stack_top(self): + """Asking for a frame above the top of the stack has no name.""" + assert get_parent_code_name(levels=10_000) == "" + + class TestGetBufferSize: """Ensure we can get the size of various buffers.""" From 9e10830cf421bc2c89bf38b834bf089a243c22b3 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 1 Aug 2026 15:33:53 +0200 Subject: [PATCH 3/8] Address bot review: nullable row fields, one history name helper Mark the two stored columns records can leave NULL (patches sample_count_total, coord_defs length) optional in the row views, share one name helper between the full and method_name history strings so unnamed callables work in both, and raise UnitError for a to_unit that is not a magnitude-1 unit instead of asserting on it. --- dascore/io/index/schema.py | 4 ++-- dascore/units.py | 4 +++- dascore/utils/patch.py | 11 +++++++---- tests/test_units.py | 10 ++++++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/dascore/io/index/schema.py b/dascore/io/index/schema.py index e6fe634d6..6fb0d6644 100644 --- a/dascore/io/index/schema.py +++ b/dascore/io/index/schema.py @@ -278,7 +278,7 @@ class PatchRow(NamedTuple): n_dims: int dims: str shape: str - sample_count_total: int + sample_count_total: int | None time_min: int | None time_max: int | None time_step: int | None @@ -295,7 +295,7 @@ class CoordDefRow(NamedTuple): fingerprint: str | None value_kind: str dtype: str - length: int + length: int | None units: str | None min_num: float | None max_num: float | None diff --git a/dascore/units.py b/dascore/units.py index 3c5a67d33..262811e06 100644 --- a/dascore/units.py +++ b/dascore/units.py @@ -382,7 +382,9 @@ def _check_to_units(to_unit, dim): _check_to_units(to_unit, dim) # get inverse of desired output units and ensure units are pure. to_quant = get_quantity(to_unit) - assert to_quant is not None and to_quant.magnitude == 1.0 + if to_quant is None or to_quant.magnitude != 1.0: + msg = f"to_unit must be a unit of magnitude 1, got {to_unit}" + raise UnitError(msg) to_units = to_quant.units quant1, quant2 = get_quantity(arg1), get_quantity(arg2) _ensure_same_units(quant1, quant2) diff --git a/dascore/utils/patch.py b/dascore/utils/patch.py index 63abbe438..515d828b9 100644 --- a/dascore/utils/patch.py +++ b/dascore/utils/patch.py @@ -76,6 +76,11 @@ def _format_values(val): return out +def _func_name(func: Callable) -> str: + """Name a callable for the history string; partials have no __name__.""" + return getattr(func, "__name__", str(func)) + + def _func_and_kwargs_str(func: Callable, patch, *args, **kwargs) -> str: """Get a str rep of the function and input args.""" # getcallargs is deprecated, but Signature.bind is not a drop-in @@ -94,9 +99,7 @@ def _func_and_kwargs_str(func: Callable, patch, *args, **kwargs) -> str: f"{k}={_format_values(v)!r}" for k, v in kwargs_.items() if v is not None ] arguments.sort() - # partials and other callables without a name still get a history entry. - name = getattr(func, "__name__", str(func)) - out = f"{name}(" + out = f"{_func_name(func)}(" if arguments: out += f"{','.join(arguments)}" return out + ")" @@ -126,7 +129,7 @@ def _get_history_str( if _history == "full": history_str = _func_and_kwargs_str(func, patch, *args, **kwargs) else: - history_str = str(func.__name__) + history_str = _func_name(func) return history_str diff --git a/tests/test_units.py b/tests/test_units.py index 43325e458..6e32a887b 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -257,6 +257,16 @@ def test_different_units_raises(self): with pytest.raises(UnitError): get_filter_units(1.0 * s, 10.0 * hz, s) + def test_impure_to_unit_raises(self): + """to_unit names a unit; a scaled quantity has no filter meaning.""" + s = get_unit("s") + match = "must be a unit of magnitude 1" + with pytest.raises(UnitError, match=match): + get_filter_units(1.0 * s, 10.0 * s, 2 * s) + + with pytest.raises(UnitError, match=match): + get_filter_units(1.0 * s, 10.0 * s, "") + def test_incompatible_units_raise(self): """The units must be the same or it should raise.""" s, m = get_unit("s"), get_unit("m") From e6e73b829f981666949f0a9fe59f1d8cafaa8a12 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 1 Aug 2026 16:30:36 +0200 Subject: [PATCH 4/8] Pin row-view field types to the schema's storage types --- tests/test_io/test_index/test_schema.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_io/test_index/test_schema.py b/tests/test_io/test_index/test_schema.py index 556b68b34..0c7730857 100644 --- a/tests/test_io/test_index/test_schema.py +++ b/tests/test_io/test_index/test_schema.py @@ -5,6 +5,7 @@ import sqlite3 from concurrent.futures import ThreadPoolExecutor from threading import Barrier +from typing import get_args, get_type_hints import pytest @@ -12,15 +13,26 @@ from dascore.io.index import get_backend from dascore.io.index.schema import INDEX_VERSION, TABLE_ROWS, TABLES +# The python type each logical storage type surfaces as. +_STORAGE_TYPES = {"int64": int, "float64": float, "str": str, "bool": bool} + class TestRowViews: """The row views index code reads must match the stored columns.""" @pytest.mark.parametrize("table", sorted(TABLE_ROWS)) def test_fields_match_columns(self, table): - """Each row view declares exactly its table's columns.""" + """Each row view declares exactly its table's columns, in order.""" assert TABLE_ROWS[table]._fields == tuple(TABLES[table]) + @pytest.mark.parametrize("table", sorted(TABLE_ROWS)) + def test_field_types_match_storage(self, table): + """Each field's type is its column's storage type, nullable or not.""" + hints = get_type_hints(TABLE_ROWS[table]) + for column, storage in TABLES[table].items(): + declared = set(get_args(hints[column])) or {hints[column]} + assert declared - {type(None)} == {_STORAGE_TYPES[storage]} + class TestSchemaValidation: """Existing files are validated without repair or implicit migration.""" From 7a0a262846e4c4ec1218fe1724b758cc156885ed Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 1 Aug 2026 17:17:30 +0200 Subject: [PATCH 5/8] Declare the index schema once, as the row classes The table column dicts and the row views said the same thing twice, kept in step by a test. Declare each table as a NamedTuple naming its columns in order and derive TABLES (the logical types the DDL is built from) from the annotations, so the row a reader sees and the columns SQLite gets cannot disagree. The per-column commentary moves onto the fields, and nullability is now recorded (as `| None`) where it previously was not. The derived mapping is identical to the literals it replaces, so the emitted DDL is unchanged. The drift tests give way to one end-to-end check that a created index's columns and types match the declaration. --- dascore/io/index/schema.py | 369 +++++++++++------------- tests/test_io/test_index/test_schema.py | 34 +-- 2 files changed, 186 insertions(+), 217 deletions(-) diff --git a/dascore/io/index/schema.py b/dascore/io/index/schema.py index 6fb0d6644..f484e3b63 100644 --- a/dascore/io/index/schema.py +++ b/dascore/io/index/schema.py @@ -1,15 +1,27 @@ """ Logical schema for the spool index. -The schema uses four primitive storage types (int64, float64, str, bool). -Times and durations are always epoch/plain nanoseconds stored as int64, -never engine-native timestamp types. +Each stored table is declared once, as a NamedTuple naming its columns in +order. The column types are the four primitive storage types (int64, +float64, str, bool), written as the python type each surfaces as and +marked with None where the column is nullable; times and durations are +always epoch/plain nanoseconds stored as int64, never engine-native +timestamp types. + +The row classes are the single source of truth: `TABLES` (the logical +column types the DDL is built from) is derived from them, and index code +reads rows through them with `iter_rows(df, Row)` (see dascore.utils.pd), +which names the row shape pandas builds dynamically in `itertuples`. They +are never instantiated. A frame holding only some of a table's columns +still uses its table's row class; only the columns actually fetched can +be read, and nullable ones may arrive as NaN rather than None (reading +code guards with `pd.isnull`). """ from __future__ import annotations from types import MappingProxyType -from typing import NamedTuple +from typing import NamedTuple, get_args, get_type_hints # Version of the index schema, independent of dascore's version. INDEX_VERSION = 4 @@ -29,126 +41,173 @@ } ) -META_DATA = MappingProxyType( - { - "what_is_this": "str", - "index_version": "int64", - "dascore_version": "str", - "last_indexed_ns": "int64", - } +# The logical storage type each declared python type maps to. +_STORAGE_TYPES = MappingProxyType( + {int: "int64", float: "float64", str: "str", bool: "bool"} ) -SOURCES = MappingProxyType( - { - "source_id": "int64", - "base_uri": "str", - "source_path": "str", - "source_format": "str", - "format_version": "str", - "mtime_ns": "int64", - "size_bytes": "int64", - # JSON dict of hive-style key=value attrs parsed from the stored - # path's directory segments; NULL when the path carries none. - # Records which attr values are path-derived so moves can rewrite - # them and patch loading can stamp them without re-parsing paths - # (derived/union catalogs absolutize paths, losing the segments). - "path_attrs": "str", - "last_indexed_ns": "int64", - # The catalog's explicit ordering contract: patch rows present in - # (ordinal, patch_id) order. Assigned at ingest (insertion - # sequence); a replaced source keeps its position while new - # sources append, so merging catalogs concatenates and - # deduplication keeps first-occurrence position with - # last-occurrence metadata (dict-merge semantics). The directory - # syncer renumbers to time order after each sync, preserving the - # conventional time-ordered presentation of file archives. - "ordinal": "int64", - } -) -# Frozen structural table; nothing dynamic is ever added here. The -# time/distance envelopes are cached summaries of the two conventional -# dims (hot path), not attr promotion. -PATCHES = MappingProxyType( - { - "patch_id": "int64", - "source_id": "int64", - "source_patch_id": "str", - "n_dims": "int64", - "dims": "str", - "shape": "str", - "sample_count_total": "int64", - "time_min": "int64", # epoch ns; NULL for relative-time patches - "time_max": "int64", - "time_step": "int64", - "distance_min": "float64", # canonical SI (m) - "distance_max": "float64", - "distance_step": "float64", - } -) +class MetaDataRow(NamedTuple): + """A row of the meta_data table (the index's identity and version).""" -# attrs table starts with only the key; typed columns (`__`) -# are added lazily at ingest. -ATTRS_BASE = MappingProxyType({"patch_id": "int64"}) + what_is_this: str + index_version: int + dascore_version: str + last_indexed_ns: int -ATTR_META = MappingProxyType( - { - "attr_name": "str", # original (unsanitized) attr name - "value_kind": "str", - "column_name": "str", # sanitized column in the attrs table - "units": "str", # canonical unit for num kinds, nullable - } -) -# Unique coordinate summaries, deduplicated across patches. Range coordinates -# use a semantic fingerprint supplied by the scan or reconstructed exactly -# from the range summary. Non-range coordinates without a fingerprint use a -# summary hash for storage deduplication, but it is not exposed as value identity. -COORD_DEFS = MappingProxyType( - { - "coord_def_id": "int64", - "def_key": "str", - "fingerprint": "str", # nullable; semantic hash from CoordSummary - "value_kind": "str", # num | time | str - "dtype": "str", - "length": "int64", - "units": "str", # original unit string; numeric values stored SI - "min_num": "float64", - "max_num": "float64", - "step_num": "float64", - "min_ns": "int64", - "max_ns": "int64", - "step_ns": "int64", - "min_str": "str", - "max_str": "str", - "is_monotonic": "bool", - "is_relative": "bool", - } -) +class SourceRow(NamedTuple): + """A row of the sources table (one scan unit).""" + + source_id: int + base_uri: str + source_path: str + source_format: str + format_version: str + mtime_ns: int | None + size_bytes: int | None + # JSON dict of hive-style key=value attrs parsed from the stored + # path's directory segments; NULL when the path carries none. + # Records which attr values are path-derived so moves can rewrite + # them and patch loading can stamp them without re-parsing paths + # (derived/union catalogs absolutize paths, losing the segments). + path_attrs: str | None + last_indexed_ns: int + # The catalog's explicit ordering contract: patch rows present in + # (ordinal, patch_id) order. Assigned at ingest (insertion + # sequence); a replaced source keeps its position while new + # sources append, so merging catalogs concatenates and + # deduplication keeps first-occurrence position with + # last-occurrence metadata (dict-merge semantics). The directory + # syncer renumbers to time order after each sync, preserving the + # conventional time-ordered presentation of file archives. + ordinal: int + + +class PatchRow(NamedTuple): + """ + A row of the patches table. + + Frozen structural table; nothing dynamic is ever added here. The + time/distance envelopes are cached summaries of the two conventional + dims (hot path), not attr promotion. + """ + + patch_id: int + source_id: int + source_patch_id: str + n_dims: int + dims: str + shape: str + sample_count_total: int | None + time_min: int | None # epoch ns; NULL for relative-time patches + time_max: int | None + time_step: int | None + distance_min: float | None # canonical SI (m) + distance_max: float | None + distance_step: float | None + + +class AttrsRow(NamedTuple): + """ + The fixed part of an attrs row. + + The table starts with only the key; typed columns (`__`) + are added lazily at ingest, so a fetched row carries more than this. + """ + + patch_id: int + + +class AttrMetaRow(NamedTuple): + """A row of the attr_meta table (one indexed attr name and kind).""" + + attr_name: str # original (unsanitized) attr name + value_kind: str + column_name: str # sanitized column in the attrs table + units: str | None # canonical unit for num kinds + + +class CoordDefRow(NamedTuple): + """ + A row of the coord_defs table. + + Unique coordinate summaries, deduplicated across patches. Range + coordinates use a semantic fingerprint supplied by the scan or + reconstructed exactly from the range summary. Non-range coordinates + without a fingerprint use a summary hash for storage deduplication, + but it is not exposed as value identity. + """ + + coord_def_id: int + def_key: str + fingerprint: str | None # semantic hash from CoordSummary + value_kind: str # num | time | str + dtype: str + length: int | None + units: str | None # original unit string; numeric values stored SI + min_num: float | None + max_num: float | None + step_num: float | None + min_ns: int | None + max_ns: int | None + step_ns: int | None + min_str: str | None + max_str: str | None + is_monotonic: bool | None + is_relative: bool | None + + +class PatchCoordRow(NamedTuple): + """ + A row of the patch_coords table. + + Links a patch to its coord defs; the name and dims are patch-level + semantics (two patches can share values under different names). + """ + + patch_id: int + coord_name: str + coord_dims: str + coord_def_id: int -# Links a patch to its coord defs; the name and dims are patch-level -# semantics (two patches can share values under different names). -PATCH_COORDS = MappingProxyType( - { - "patch_id": "int64", - "coord_name": "str", - "coord_dims": "str", - "coord_def_id": "int64", - } -) -TABLES = MappingProxyType( +# The row class declaring each stored table. +TABLE_ROWS = MappingProxyType( { - "meta_data": META_DATA, - "sources": SOURCES, - "patches": PATCHES, - "attrs": ATTRS_BASE, - "attr_meta": ATTR_META, - "coord_defs": COORD_DEFS, - "patch_coords": PATCH_COORDS, + "meta_data": MetaDataRow, + "sources": SourceRow, + "patches": PatchRow, + "attrs": AttrsRow, + "attr_meta": AttrMetaRow, + "coord_defs": CoordDefRow, + "patch_coords": PatchCoordRow, } ) + +def _columns(row_type: type[NamedTuple]) -> MappingProxyType[str, str]: + """Return a row class's {column: logical storage type} mapping.""" + out = {} + for name, hint in get_type_hints(row_type).items(): + # Nullability is not part of the storage type; a nullable column + # is declared as ` | None` for readers of the row. + types = set(get_args(hint)) - {type(None)} or {hint} + assert len(types) == 1, f"{row_type.__name__}.{name} needs one storage type" + out[name] = _STORAGE_TYPES[types.pop()] + return MappingProxyType(out) + + +TABLES = MappingProxyType({name: _columns(row) for name, row in TABLE_ROWS.items()}) + +META_DATA = TABLES["meta_data"] +SOURCES = TABLES["sources"] +PATCHES = TABLES["patches"] +ATTRS_BASE = TABLES["attrs"] +ATTR_META = TABLES["attr_meta"] +COORD_DEFS = TABLES["coord_defs"] +PATCH_COORDS = TABLES["patch_coords"] + # Keeping constraints beside the logical columns makes the stored contract # explicit and keeps dynamic attr-column DDL separate from table identity. TABLE_CONSTRAINTS = MappingProxyType( @@ -243,92 +302,6 @@ # non-private columns. SPOOL_HIDDEN_COLUMNS = ("n_dims", "sample_count_total", "shape") -# --- Row views ------------------------------------------------------- -# -# Index code reads table rows with `iter_rows(df, Row)` (see -# dascore.utils.pd), which names the row shape pandas builds dynamically -# in `itertuples`. Fields mirror the table definitions above (a test keeps -# them in step) and nullable columns are typed with None, though pandas -# may surface them as NaN — reading code guards with `pd.isnull`. A frame -# holding only some of a table's columns still uses its table's row view; -# only the columns actually fetched can be read. - - -class SourceRow(NamedTuple): - """A row of the sources table.""" - - source_id: int - base_uri: str - source_path: str - source_format: str - format_version: str - mtime_ns: int | None - size_bytes: int | None - path_attrs: str | None - last_indexed_ns: int - ordinal: int - - -class PatchRow(NamedTuple): - """A row of the patches table.""" - - patch_id: int - source_id: int - source_patch_id: str - n_dims: int - dims: str - shape: str - sample_count_total: int | None - time_min: int | None - time_max: int | None - time_step: int | None - distance_min: float | None - distance_max: float | None - distance_step: float | None - - -class CoordDefRow(NamedTuple): - """A row of the coord_defs table.""" - - coord_def_id: int - def_key: str - fingerprint: str | None - value_kind: str - dtype: str - length: int | None - units: str | None - min_num: float | None - max_num: float | None - step_num: float | None - min_ns: int | None - max_ns: int | None - step_ns: int | None - min_str: str | None - max_str: str | None - is_monotonic: bool | None - is_relative: bool | None - - -class PatchCoordRow(NamedTuple): - """A row of the patch_coords table.""" - - patch_id: int - coord_name: str - coord_dims: str - coord_def_id: int - - -# Row view for each stored table, used to check the views stay in step -# with the column definitions. -TABLE_ROWS = MappingProxyType( - { - "sources": SourceRow, - "patches": PatchRow, - "coord_defs": CoordDefRow, - "patch_coords": PatchCoordRow, - } -) - # Explicit secondary indexes. Every other access path is covered by a # PRIMARY KEY or UNIQUE autoindex above — patch_coords(patch_id, # coord_name), sources(base_uri, source_path), patches(source_id, diff --git a/tests/test_io/test_index/test_schema.py b/tests/test_io/test_index/test_schema.py index 0c7730857..e0b83e772 100644 --- a/tests/test_io/test_index/test_schema.py +++ b/tests/test_io/test_index/test_schema.py @@ -5,33 +5,29 @@ import sqlite3 from concurrent.futures import ThreadPoolExecutor from threading import Barrier -from typing import get_args, get_type_hints import pytest from dascore.exceptions import InvalidIndexError, InvalidIndexVersionError from dascore.io.index import get_backend -from dascore.io.index.schema import INDEX_VERSION, TABLE_ROWS, TABLES +from dascore.io.index.schema import INDEX_VERSION, TABLES -# The python type each logical storage type surfaces as. -_STORAGE_TYPES = {"int64": int, "float64": float, "str": str, "bool": bool} +class TestSchemaDeclaration: + """The row classes are the schema; check they reach SQLite intact.""" -class TestRowViews: - """The row views index code reads must match the stored columns.""" - - @pytest.mark.parametrize("table", sorted(TABLE_ROWS)) - def test_fields_match_columns(self, table): - """Each row view declares exactly its table's columns, in order.""" - assert TABLE_ROWS[table]._fields == tuple(TABLES[table]) - - @pytest.mark.parametrize("table", sorted(TABLE_ROWS)) - def test_field_types_match_storage(self, table): - """Each field's type is its column's storage type, nullable or not.""" - hints = get_type_hints(TABLE_ROWS[table]) - for column, storage in TABLES[table].items(): - declared = set(get_args(hints[column])) or {hints[column]} - assert declared - {type(None)} == {_STORAGE_TYPES[storage]} + def test_stored_columns_match_declaration(self, tmp_path): + """A created index has each table's declared columns and types.""" + backend = get_backend(tmp_path / "index.sqlite3") + dialect = backend.dialect + for table, columns in TABLES.items(): + info = backend._con.execute(f'PRAGMA table_info("{table}")').fetchall() + stored = {row[1]: row[2] for row in info} + expected = { + name: dialect.type_map[logical] for name, logical in columns.items() + } + assert stored == expected + backend.close() class TestSchemaValidation: From 827c8f1b38b08d3de502afee538fd998f2b474b5 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Mon, 3 Aug 2026 22:29:34 +0200 Subject: [PATCH 6/8] Finish the unresolved-attribute burn-down in coords and spool Answer the two API questions the rule raised instead of holding the files back with a per-file override. A PatchCatalog without a resolver holds rows it can never resolve, and all six construction paths already passed one, so resolver becomes a required keyword argument and the None branches it justified go away. Spool._catalog is declared as a PatchCatalog rather than defaulting to None; __init__ sets it on every path (copy-construction and unpickling included), so its None guards go with it. BaseCoord promises _get_index, which now has a base implementation: get_next_index on a sorted string coord raised AttributeError, and string coords have no value spacing to index into, so it raises CoordError like the other unsupported string-coord operations. start and stop stay CoordRange's, so approx_equal narrows with isinstance. Also fix the sorted/evenly_sampled/reverse_sorted return annotations (they return bools, not tuples) and cover the NaT timedelta envelope. --- dascore/core/coords.py | 24 ++++++++++++++++++------ dascore/core/spool.py | 17 +++++++++-------- dascore/io/index/catalog.py | 7 +++---- pyproject.toml | 14 ++------------ tests/test_core/test_coords.py | 7 +++++++ tests/test_io/test_index/test_planned.py | 7 +++++++ 6 files changed, 46 insertions(+), 30 deletions(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index 16042d93f..b160dffb0 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -629,17 +629,17 @@ def size(self) -> int: return np.prod(self.shape) @property - def evenly_sampled(self) -> tuple[int, ...]: + def evenly_sampled(self) -> bool: """Returns True if the coord is evenly sampled.""" return self._evenly_sampled @property - def sorted(self) -> tuple[int, ...]: + def sorted(self) -> bool: """Returns True if the coord in sorted.""" return self._sorted @property - def reverse_sorted(self) -> tuple[int, ...]: + def reverse_sorted(self) -> bool: """Returns True if the coord in sorted in reverse order.""" return self._reverse_sorted @@ -1006,6 +1006,16 @@ def get_sample_count(self, value, samples=False, enforce_lt_coord=False) -> int: raise ParameterError(msg) return samples + def _get_index(self, value, forward=True): + """ + Get the index a value would occupy in the coordinate. + + Overridden by the coords that can search their values; the rest + (unordered arrays, string coords) have no such position to report. + """ + msg = f"{type(self).__name__} does not support indexing by value." + raise CoordError(msg) + def get_next_index( self, value, samples=False, allow_out_of_bounds=False, relative=False ) -> int: @@ -1107,9 +1117,10 @@ def approx_equal(self: BaseCoord, other: BaseCoord) -> bool: return self == other if any(non_coords): return False - # Evenly sampled coords with identical start/stop/step have identical - # values; this avoids materializing and comparing the value arrays. - if self._evenly_sampled and other._evenly_sampled: + # Ranges (the evenly sampled coords) with identical start/stop/step + # have identical values; this avoids materializing and comparing + # the value arrays. + if isinstance(self, CoordRange) and isinstance(other, CoordRange): same = ( self.start == other.start and self.stop == other.stop @@ -2265,6 +2276,7 @@ def select( sub, seg_lo, seg_hi = seg, 0, len(seg) else: # boundary segment; delegate the exact trim sub, indexer = seg.select((v1, v2)) + assert isinstance(indexer, slice) # a value window is contiguous seg_lo, seg_hi, _ = indexer.indices(len(seg)) if seg_hi <= seg_lo: continue diff --git a/dascore/core/spool.py b/dascore/core/spool.py index a63e7f86b..69a7c4661 100644 --- a/dascore/core/spool.py +++ b/dascore/core/spool.py @@ -6,7 +6,7 @@ from collections.abc import Callable, Generator, Sequence from functools import singledispatch from pathlib import Path -from typing import ClassVar, Literal, TypeVar +from typing import TYPE_CHECKING, ClassVar, Literal, TypeVar import numpy as np import pandas as pd @@ -44,6 +44,9 @@ ) from dascore.utils.paths import coerce_to_upath, requires_local_directory +if TYPE_CHECKING: + from dascore.io.index.catalog import PatchCatalog + T = TypeVar("T") @@ -458,8 +461,8 @@ class Spool(BaseSpool): # synthetic catalog identity columns must not join patch kwargs # comparisons or chunk merge-compatibility checks _drop_columns = ("patch", "path", "file_format", "file_version", "source_patch_id") - # The catalog backing this spool. - _catalog = None + # The catalog backing this spool; every construction path sets one. + _catalog: PatchCatalog # single-file provenance (set by from_file; drives update()) _file_path = None _file_format = None @@ -539,7 +542,6 @@ def __getitem__(self, item) -> PatchType | BaseSpool: def __iter__(self): # The catalog snapshots the relation once and skips patches which # cannot be resolved (see #583). - assert self._catalog is not None # __init__ always sets the catalog yield from self._catalog # --- selection and presentation specs ------------------------------- @@ -915,7 +917,7 @@ def from_file( @property def indexer(self): """The directory syncer, or None for non-directory spools.""" - return None if self._catalog is None else self._catalog._syncer + return self._catalog._syncer @property def spool_path(self): @@ -930,8 +932,7 @@ def spool_path(self): @property def has_live_patches(self) -> bool: """True when any of this spool's patches live in memory.""" - catalog = self._catalog - return catalog is not None and bool(catalog.resolver.live_entries()) + return bool(self._catalog.resolver.live_entries()) @compose_docstring(doc=BaseSpool.update.__doc__) def update(self, progress: PROGRESS_LEVELS = "standard") -> Self: @@ -954,7 +955,7 @@ def update(self, progress: PROGRESS_LEVELS = "standard") -> Self: "Update the root spool and re-apply the operations, e.g. " "root = root.update(); view = root.select(...)." ) - if catalog is None or catalog.is_view: + if catalog.is_view: raise InvalidSpoolError(derived_msg) if catalog._syncer is not None: catalog.update(progress=progress) diff --git a/dascore/io/index/catalog.py b/dascore/io/index/catalog.py index 8f756ac81..65525c68c 100644 --- a/dascore/io/index/catalog.py +++ b/dascore/io/index/catalog.py @@ -485,8 +485,8 @@ class PatchCatalog: def __init__( self, *, + resolver: PatchResolver, backend=None, - resolver: PatchResolver | None = None, syncer=None, queries: tuple[Query, ...] = (), residuals: tuple[tuple[dict, bool], ...] = (), @@ -689,7 +689,7 @@ def __getstate__(self) -> dict: # presentation order, so a rebuilt registry keeps the view's # ordering. resolver = self.resolver - if self.is_view and resolver is not None and resolver.live_entries(): + if self.is_view and resolver.live_entries(): df = self.to_df() paths = list(dict.fromkeys(df["path"].astype(str))) entries = resolver.live_entries() @@ -976,7 +976,6 @@ def resolve_row(self, row: Mapping, extra_trim: Mapping | None = None) -> dc.Pat } ) trim_hint.update(extra_trim or {}) - assert self.resolver is not None # rows only exist once one is set patch = self.resolver.resolve(row, **trim_hint) return apply_exact_residuals(patch, self._residuals) @@ -1039,7 +1038,7 @@ def remove(self, source_paths: Sequence[str], base_uri: str = "") -> PatchCatalo self.backend.delete_sources(source_paths, base_uri=base_uri) # The live registry is the store for in-memory patches; it must # stay in step with the backend rows (pickling rebuilds from it). - registry = self.resolver.live_entries() if self.resolver else {} + registry = self.resolver.live_entries() for path in source_paths: registry.pop(path, None) self._invalidate() diff --git a/pyproject.toml b/pyproject.toml index a19de70b6..0d9d47bc6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -260,8 +260,8 @@ line-ending = "lf" include = ["dascore"] # tests/ still has many diagnostics; expand scope later. # Rules with large pre-existing error counts, ignored until incrementally -# burned down. Counts as of 2026-08-01: invalid-argument-type 175, -# invalid-return-type 72, invalid-method-override 36, no-matching-overload 13, +# burned down. Counts as of 2026-08-03: invalid-argument-type 171, +# invalid-return-type 69, invalid-method-override 36, no-matching-overload 13, # not-subscriptable 5. [tool.ty.rules] invalid-argument-type = "ignore" @@ -278,16 +278,6 @@ include = ["dascore/compat.py", "dascore/utils/jit.py", "dascore/io/dasvader/uti [tool.ty.overrides.rules] unresolved-import = "ignore" -# The last 26 unresolved-attribute errors are two API questions rather than -# wrong hints: which members BaseCoord promises (start/stop/_get_index) and -# whether a PatchCatalog can exist without a backing catalog/resolver. Both -# are settled, with the rest of the coord/spool API, in a follow-up PR. -[[tool.ty.overrides]] -include = ["dascore/core/coords.py", "dascore/core/spool.py"] - -[tool.ty.overrides.rules] -unresolved-attribute = "ignore" - [tool.typos.files] extend-exclude = ["docs/_static/logo.svg"] diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index fee555613..a041058ae 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -2027,6 +2027,13 @@ def test_out_of_bounds_suppressed(self, evenly_sampled_coord): expected_above = int((max_value + step - min_value) / step) assert above_max_idx == expected_above # Should be len(coord) + def test_string_coord_raises(self): + """Sorted strings have no value spacing to index into.""" + coord = get_coord(values=np.array(["a", "b", "c"])) + assert coord.sorted + with pytest.raises(CoordError, match="does not support indexing by value"): + coord.get_next_index("b") + def test_exact_values(self, evenly_sampled_coord): """Ensure using exact values contained in coord return index.""" coord = evenly_sampled_coord diff --git a/tests/test_io/test_index/test_planned.py b/tests/test_io/test_index/test_planned.py index fdc375ea1..a41e9f728 100644 --- a/tests/test_io/test_index/test_planned.py +++ b/tests/test_io/test_index/test_planned.py @@ -47,6 +47,13 @@ def test_coord_record_numpy_datetimes(self): assert record.value_kind == "time" assert record.min_ns == _ns(lo) + def test_coord_record_half_null_timedelta(self): + """A one-sided timedelta envelope keeps NaT rather than raising.""" + row = {"time_min": pd.Timedelta(seconds=1), "time_max": pd.NaT} + record = _coord_record_from_row(row, "time") + assert record.min_ns == pd.Timedelta(seconds=1).value + assert pd.isnull(np.timedelta64(record.max_ns, "ns")) + def test_coord_record_zero_step_length(self): """A degenerate step leaves length unknown instead of raising.""" row = {"time_min": 0.0, "time_max": 1.0, "time_step": 0.0} From dcfb823977714beab94d32537ef1e3384cb64901 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Mon, 3 Aug 2026 22:29:34 +0200 Subject: [PATCH 7/8] Name insert columns with the schema's row classes The per-table alias constants were a third name for what the row classes and TABLES already say; three of the seven had no users left. Inserts now take their column list from the row class whose tuples they are inserting. --- dascore/io/index/backend.py | 16 ++++++++-------- dascore/io/index/schema.py | 10 ++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/dascore/io/index/backend.py b/dascore/io/index/backend.py index c8643b671..c486a9cb0 100644 --- a/dascore/io/index/backend.py +++ b/dascore/io/index/backend.py @@ -38,16 +38,16 @@ build_sql, ) from dascore.io.index.schema import ( - COORD_DEFS, INDEX_VERSION, INDEXES, KIND_STORAGE, - PATCH_COORDS, - PATCHES, - SOURCES, TABLE_CONSTRAINTS, TABLES, WHAT_IS_THIS, + CoordDefRow, + PatchCoordRow, + PatchRow, + SourceRow, ) from dascore.units import convert_units from dascore.utils.pd import resolve_selector_namespaces @@ -404,7 +404,7 @@ def _ensure_coord_defs(self, defs_needed: dict) -> dict[str, int]: ) mapping[key] = next_id next_id += 1 - self._bulk_insert("coord_defs", tuple(COORD_DEFS), def_rows) + self._bulk_insert("coord_defs", CoordDefRow._fields, def_rows) return mapping def _bulk_insert(self, table: str, columns: tuple, rows: list) -> None: @@ -511,14 +511,14 @@ def write_sources(self, records: list[SourceRecord]) -> None: link_rows.append((patch_id, c.coord_name, c.coord_dims, key)) patch_id += 1 source_id += 1 - self._bulk_insert("sources", tuple(SOURCES), source_rows) - self._bulk_insert("patches", tuple(PATCHES), patch_rows) + self._bulk_insert("sources", SourceRow._fields, source_rows) + self._bulk_insert("patches", PatchRow._fields, patch_rows) for columns, rows in attr_groups.items(): self._bulk_insert("attrs", ("patch_id", *columns), rows) def_ids = self._ensure_coord_defs(defs_needed) self._bulk_insert( "patch_coords", - tuple(PATCH_COORDS), + PatchCoordRow._fields, [(pid, name, dims, def_ids[key]) for pid, name, dims, key in link_rows], ) # meta_data.last_indexed_ns is the initial-update-complete diff --git a/dascore/io/index/schema.py b/dascore/io/index/schema.py index f484e3b63..195f39cd1 100644 --- a/dascore/io/index/schema.py +++ b/dascore/io/index/schema.py @@ -198,16 +198,10 @@ def _columns(row_type: type[NamedTuple]) -> MappingProxyType[str, str]: return MappingProxyType(out) +# The logical columns of each table, in declaration order; the DDL and +# every insert's column list are built from these. TABLES = MappingProxyType({name: _columns(row) for name, row in TABLE_ROWS.items()}) -META_DATA = TABLES["meta_data"] -SOURCES = TABLES["sources"] -PATCHES = TABLES["patches"] -ATTRS_BASE = TABLES["attrs"] -ATTR_META = TABLES["attr_meta"] -COORD_DEFS = TABLES["coord_defs"] -PATCH_COORDS = TABLES["patch_coords"] - # Keeping constraints beside the logical columns makes the stored contract # explicit and keeps dynamic attr-column DDL separate from table identity. TABLE_CONSTRAINTS = MappingProxyType( From 8a2d60af5d761e12a7f684513cb31f06c71ae53a Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Mon, 3 Aug 2026 22:35:36 +0200 Subject: [PATCH 8/8] Correct two comments flagged in review String coords do have lexicographic insertion positions; what they lack is positional semantics, by policy. And iter_rows names the row shape for readers and type checkers only, not at runtime. --- dascore/core/coords.py | 5 +++-- dascore/io/index/schema.py | 5 +++-- tests/test_core/test_coords.py | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dascore/core/coords.py b/dascore/core/coords.py index b160dffb0..1ca3b070e 100644 --- a/dascore/core/coords.py +++ b/dascore/core/coords.py @@ -1010,8 +1010,9 @@ def _get_index(self, value, forward=True): """ Get the index a value would occupy in the coordinate. - Overridden by the coords that can search their values; the rest - (unordered arrays, string coords) have no such position to report. + Overridden by the coords that index by value. Unordered arrays + have no such position, and string coords deliberately keep out of + positional semantics (see _raise_string_coord_error). """ msg = f"{type(self).__name__} does not support indexing by value." raise CoordError(msg) diff --git a/dascore/io/index/schema.py b/dascore/io/index/schema.py index 195f39cd1..2df303adb 100644 --- a/dascore/io/index/schema.py +++ b/dascore/io/index/schema.py @@ -11,8 +11,9 @@ The row classes are the single source of truth: `TABLES` (the logical column types the DDL is built from) is derived from them, and index code reads rows through them with `iter_rows(df, Row)` (see dascore.utils.pd), -which names the row shape pandas builds dynamically in `itertuples`. They -are never instantiated. A frame holding only some of a table's columns +which names — for readers and type checkers, not at runtime — the row +shape pandas builds dynamically in `itertuples`. The classes are never +instantiated. A frame holding only some of a table's columns still uses its table's row class; only the columns actually fetched can be read, and nullable ones may arrive as NaN rather than None (reading code guards with `pd.isnull`). diff --git a/tests/test_core/test_coords.py b/tests/test_core/test_coords.py index a041058ae..41093c2fa 100644 --- a/tests/test_core/test_coords.py +++ b/tests/test_core/test_coords.py @@ -2028,7 +2028,7 @@ def test_out_of_bounds_suppressed(self, evenly_sampled_coord): assert above_max_idx == expected_above # Should be len(coord) def test_string_coord_raises(self): - """Sorted strings have no value spacing to index into.""" + """String coords stay out of positional semantics, sorted or not.""" coord = get_coord(values=np.array(["a", "b", "c"])) assert coord.sorted with pytest.raises(CoordError, match="does not support indexing by value"):