From 730546464bfe28d2f5d23b4dc8dcde5efd15e062 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:37:41 +1000 Subject: [PATCH 1/6] feat(dtypes): safe-eval dtype-repr parser + InvalidBackendTypeError (item 54, task 1) --- src/mountainash/core/dtypes/__init__.py | 9 +- src/mountainash/core/dtypes/_paramstring.py | 81 ++++++++++++++ src/mountainash/core/dtypes/errors.py | 18 ++++ tests/core/dtypes/test_paramstring.py | 112 ++++++++++++++++++++ 4 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 src/mountainash/core/dtypes/_paramstring.py create mode 100644 tests/core/dtypes/test_paramstring.py diff --git a/src/mountainash/core/dtypes/__init__.py b/src/mountainash/core/dtypes/__init__.py index 1adbb738..ebd80739 100644 --- a/src/mountainash/core/dtypes/__init__.py +++ b/src/mountainash/core/dtypes/__init__.py @@ -13,7 +13,12 @@ parse_dtype, ) from .casts import SAFE_CASTS, UNSAFE_CASTS, CastSafety, classify_cast, is_safe_cast -from .errors import DtypeError, DtypeMappingError, UnknownDtypeError +from .errors import ( + DtypeError, + DtypeMappingError, + InvalidBackendTypeError, + UnknownDtypeError, +) from .registry import DtypeRegistry, registry from .targets import TypeTarget, detect_target @@ -23,7 +28,7 @@ "parse_dtype", "parse_cast_target", "TypeTarget", "detect_target", "DtypeRegistry", "registry", - "DtypeError", "UnknownDtypeError", "DtypeMappingError", + "DtypeError", "UnknownDtypeError", "DtypeMappingError", "InvalidBackendTypeError", "SAFE_CASTS", "UNSAFE_CASTS", "is_safe_cast", "CastSafety", "classify_cast", ] diff --git a/src/mountainash/core/dtypes/_paramstring.py b/src/mountainash/core/dtypes/_paramstring.py new file mode 100644 index 00000000..567aeb13 --- /dev/null +++ b/src/mountainash/core/dtypes/_paramstring.py @@ -0,0 +1,81 @@ +"""Shared safe parser for constructor-call-style dtype repr strings. + +Polars and Narwhals dtype reprs (``str(pl.Datetime(time_unit='us', +time_zone='UTC'))``, ``str(pl.List(pl.Int64))``, +``str(nw.Enum(categories=['a','b']))``) are valid Python constructor-call +syntax against their own module namespace. This walks the AST and evaluates +ONLY a bounded grammar — a bare Name (resolved against ``namespace``), or a +Call(Name, *args, **kwargs) whose arguments are themselves only +Constant / Name (resolved against the SAME ``namespace``) / list[arg] / +tuple[arg] — before ever calling ``eval()``. Anything else (attribute access, +subscripts, comprehensions, imports, starred/double-starred unpacking, +arbitrary calls) is rejected before eval ever runs. + +GLM-5.2 review (2026-08-16) Critical finding: an earlier draft's +``_safe_literal`` accepted only Constant/List/Tuple, rejecting a bare +``Name`` inside ``args`` — which broke ``List(Int64)`` and +``Array(Int64, shape=(5,))``, the exact motivating round-trip cases +(``Int64``/``shape`` values are bare Names in the AST, not Constants). Fixed +here: a Name node inside an argument position resolves against the same +closed ``namespace`` the top-level Call already validated against — never a +wider lookup. +""" +from __future__ import annotations + +import ast +from typing import Any, Mapping, Optional + + +def _safe_arg(node: ast.AST, namespace: Mapping[str, Any]) -> Any: + if isinstance(node, ast.Constant): + return node.value + if isinstance(node, ast.Name): + if node.id not in namespace: + raise ValueError(f"name {node.id!r} not in whitelist") + return namespace[node.id] + if isinstance(node, (ast.List, ast.Tuple)): + vals = [_safe_arg(e, namespace) for e in node.elts] + return vals if isinstance(node, ast.List) else tuple(vals) + raise ValueError(f"unsupported argument node: {ast.dump(node)}") + + +def parse_constructor_repr(s: str, namespace: Mapping[str, Any]) -> Optional[Any]: + """Parse `s` as a bare name or constructor call against `namespace`. + + Returns the constructed value, or None if `s` doesn't match the bounded + grammar or references a name outside `namespace`. Never calls eval()/exec() + on unvalidated input — the AST is walked and every node type-checked first. + + Note: `**dict`-unpacked keywords (`ast.keyword(arg=None)`, e.g. + `Datetime(**{'time_zone': 'UTC'})`) are deliberately DROPPED, not rejected — + real Polars/Narwhals `str()` output never emits this form, so it is + robustness-only; §7.2 adversarially tests that the drop (not a silent + wrong-value construction) is what happens. + """ + try: + tree = ast.parse(s, mode="eval") + except SyntaxError: + return None + node = tree.body + + if isinstance(node, ast.Name): + return namespace.get(node.id) + + if isinstance(node, ast.Call): + if not isinstance(node.func, ast.Name) or node.func.id not in namespace: + return None + target = namespace[node.func.id] + try: + args = [_safe_arg(a, namespace) for a in node.args] + kwargs = { + kw.arg: _safe_arg(kw.value, namespace) + for kw in node.keywords if kw.arg # kw.arg is None for **unpack — dropped + } + except ValueError: + return None + try: + return target(*args, **kwargs) + except Exception: + return None + + return None diff --git a/src/mountainash/core/dtypes/errors.py b/src/mountainash/core/dtypes/errors.py index 6f44b33d..e6210505 100644 --- a/src/mountainash/core/dtypes/errors.py +++ b/src/mountainash/core/dtypes/errors.py @@ -6,8 +6,13 @@ """ from __future__ import annotations +from typing import TYPE_CHECKING + from mountainash.core.errors import MountainashError +if TYPE_CHECKING: + from .targets import TypeTarget + class DtypeError(MountainashError, ValueError): """Base for canonical dtype-system errors.""" @@ -19,3 +24,16 @@ class UnknownDtypeError(DtypeError): class DtypeMappingError(DtypeError): """The canonical dtype has no mapping for the requested target/use.""" + + +class InvalidBackendTypeError(DtypeError): + """A non-empty FieldSpec.backend_type could not be parsed for the target.""" + + def __init__(self, field_name: str, backend_type: str, target: "TypeTarget") -> None: + self.field_name = field_name + self.backend_type = backend_type + self.target = target + super().__init__( + f"field {field_name!r}: backend_type {backend_type!r} is not a valid " + f"{target.value} dtype string" + ) diff --git a/tests/core/dtypes/test_paramstring.py b/tests/core/dtypes/test_paramstring.py new file mode 100644 index 00000000..16ec9101 --- /dev/null +++ b/tests/core/dtypes/test_paramstring.py @@ -0,0 +1,112 @@ +"""Unit tests for the restricted AST-validated dtype-repr parser. + +Security-relevant surface: ``parse_constructor_repr`` must NEVER evaluate +arbitrary input — only a bounded grammar (bare Name, or Call(Name, args) +whose arguments are Constant / whitelisted-Name / list / tuple). Every +adversarial case below must return None, never raise or execute. + +Positive cases cover the Critical-finding fix: a bare ``Name`` used as a +positional/keyword argument (``List(Int64)``, ``Array(Int64, shape=(5,))``) +resolves against the SAME closed namespace, not a wider lookup. +""" +from __future__ import annotations + +import pytest + +from mountainash.core.dtypes._paramstring import parse_constructor_repr + + +class _Box: + """Minimal stand-in for a dtype constructor: records its arguments.""" + + def __init__(self, *args, **kwargs) -> None: + self.args = args + self.kwargs = kwargs + + +# Closed namespace mirroring the shape of the polars/narwhals whitelists. +_NS = { + "Datetime": _Box, + "List": _Box, + "Array": _Box, + "Enum": _Box, + "Int64": "INT64", + "shape": "SHAPE", +} + + +class TestPositive: + def test_bare_name(self): + assert parse_constructor_repr("Int64", _NS) == "INT64" + + def test_bare_name_arg_resolves_against_namespace(self): + """The Critical-finding case: Name args must resolve, not be rejected.""" + result = parse_constructor_repr("List(Int64)", _NS) + assert isinstance(result, _Box) + assert result.args == ("INT64",) + + def test_name_arg_plus_tuple_of_constants_kwarg(self): + result = parse_constructor_repr("Array(Int64, shape=(5,))", _NS) + assert isinstance(result, _Box) + assert result.args == ("INT64",) + assert result.kwargs == {"shape": (5,)} + + def test_all_constant_kwargs(self): + result = parse_constructor_repr( + "Datetime(time_unit='us', time_zone='UTC')", _NS + ) + assert isinstance(result, _Box) + assert result.kwargs == {"time_unit": "us", "time_zone": "UTC"} + + def test_list_arg(self): + result = parse_constructor_repr("Enum(categories=['a', 'b'])", _NS) + assert isinstance(result, _Box) + assert result.kwargs == {"categories": ["a", "b"]} + + +class TestNegative: + """Everything outside the bounded grammar returns None — never executes.""" + + @pytest.mark.parametrize("s", [ + "os.system('x')", # attribute access + "eval('1')", # arbitrary call + "Datetime.__init__()", # method call + "(lambda: 1)()", # lambda + "[x for x in [1, 2]]", # comprehension (also not a valid expr form) + "__import__('os')", # import builtin + "Datetime(*[1, 2])", # starred unpacking + "List(__builtins__)", # Name outside whitelist in arg position + "1 + 1", # operator expression + "Datetime.time_unit", # attribute access as target + ]) + def test_rejected_returns_none(self, s): + assert parse_constructor_repr(s, _NS) is None + + def test_unparseable_syntax_returns_none(self): + assert parse_constructor_repr("not valid python !!!", _NS) is None + + def test_empty_string_returns_none(self): + assert parse_constructor_repr("", _NS) is None + + def test_unknown_top_level_name_returns_none(self): + assert parse_constructor_repr("NotInNamespace", _NS) is None + assert parse_constructor_repr("NotInNamespace(1)", _NS) is None + + +class TestDoubleStarKwargDrop: + def test_unpacked_kwargs_are_dropped_not_applied(self): + """Deliberate, documented behavior: **dict-unpacked keywords are + dropped (real Polars/Narwhals str() output never emits this form).""" + result = parse_constructor_repr( + "Datetime(time_unit='us', **{'time_zone': 'UTC'})", _NS + ) + assert isinstance(result, _Box) + assert result.kwargs == {"time_unit": "us"} + assert "time_zone" not in result.kwargs + + def test_fully_unpacked_kwargs_construct_with_nothing(self): + """All-keywords-unpacked form drops everything — constructs with zero + kwargs, never rejects (matches the documented drop, not a raise).""" + result = parse_constructor_repr("Datetime(**{'a': 1})", _NS) + assert isinstance(result, _Box) + assert result.kwargs == {} From 39742c5ca9ca5c014432b22de66eea14b12879a6 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:38:22 +1000 Subject: [PATCH 2/6] feat(dtypes): polars parameterized backend_type parsing (item 54, task 2) --- src/mountainash/core/dtypes/target_polars.py | 19 ++++-- tests/core/dtypes/test_target_modules.py | 11 +++- tests/core/dtypes/test_target_polars.py | 67 ++++++++++++++++++++ 3 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 tests/core/dtypes/test_target_polars.py diff --git a/src/mountainash/core/dtypes/target_polars.py b/src/mountainash/core/dtypes/target_polars.py index e9044195..9a1647dd 100644 --- a/src/mountainash/core/dtypes/target_polars.py +++ b/src/mountainash/core/dtypes/target_polars.py @@ -52,9 +52,18 @@ def from_native(native: Any) -> Optional[D]: def parse_type_string(s: str) -> Optional[Any]: - if "(" in s: # parameterized reprs not reconstructable via getattr - return None - t = getattr(pl, s, None) - if isinstance(t, type) and issubclass(t, pl.DataType): - return t + if "(" not in s: + t = getattr(pl, s, None) + return t if isinstance(t, type) and issubclass(t, pl.DataType) else None + from ._paramstring import parse_constructor_repr + namespace = { + n: getattr(pl, n) for n in + ("Datetime", "Duration", "Decimal", "List", "Array", "Int8", "Int16", + "Int32", "Int64", "UInt8", "UInt16", "UInt32", "UInt64", "Float32", + "Float64", "Boolean", "String", "Binary", "Date", "Time", + "Categorical", "Enum") + } + result = parse_constructor_repr(s, namespace) + if result is not None and isinstance(result, pl.DataType): + return result return None diff --git a/tests/core/dtypes/test_target_modules.py b/tests/core/dtypes/test_target_modules.py index 6f282615..64b2bb43 100644 --- a/tests/core/dtypes/test_target_modules.py +++ b/tests/core/dtypes/test_target_modules.py @@ -46,8 +46,15 @@ def test_from_native_unknown_raises(self): def test_parse_type_string_bare_name(self): assert target_polars.parse_type_string("Int32") is pl.Int32 - def test_parse_type_string_parameterized_returns_none(self): - assert target_polars.parse_type_string("Datetime(time_unit='us', time_zone=None)") is None + def test_parse_type_string_parameterized_round_trips(self): + # Parameterized reprs now reconstruct via the safe-eval parser + # (item 54, gap 1) — no longer dropped to canonical fallback. + assert target_polars.parse_type_string( + "Datetime(time_unit='us', time_zone=None)" + ) == pl.Datetime(time_unit="us", time_zone=None) + + def test_parse_type_string_garbage_returns_none(self): + assert target_polars.parse_type_string("garbage") is None class TestPandas: diff --git a/tests/core/dtypes/test_target_polars.py b/tests/core/dtypes/test_target_polars.py new file mode 100644 index 00000000..ca9b23d0 --- /dev/null +++ b/tests/core/dtypes/test_target_polars.py @@ -0,0 +1,67 @@ +"""Polars parse_type_string — parameterized backend_type fidelity (item 54, gap 1). + +Round-trips: str(pl.) output is valid constructor-call syntax against +the polars namespace; parse_type_string must reconstruct the real +parameterized dtype instead of returning None (canonical fallback). +""" +from __future__ import annotations + +import polars as pl +import pytest + +from mountainash.core.dtypes import target_polars + + +class TestParameterizedRoundTrip: + @pytest.mark.parametrize("s,expected", [ + ("Datetime(time_unit='us', time_zone='UTC')", + pl.Datetime(time_unit="us", time_zone="UTC")), + ("Duration(time_unit='ms')", pl.Duration(time_unit="ms")), + # NOTE: MountainashDtype has no canonical DECIMAL member + # (canonical.py) — backend_type is Decimal's ONLY path to a schema + # entry on every target; there is no canonical fallback to fall back + # to. That is why this test has no non-backend_type counterpart. + ("Decimal(precision=38, scale=10)", pl.Decimal(precision=38, scale=10)), + ("List(Int64)", pl.List(pl.Int64)), + ]) + def test_parameterized_repr_round_trips(self, s, expected): + assert target_polars.parse_type_string(s) == expected + + def test_array_with_bare_name_arg_and_shape_tuple(self): + """Critical-finding case: bare-Name arg (Int64) + tuple-of-Constant + kwarg (shape=(5,)) — the naive Name-rejecting implementation fails + this one explicitly.""" + result = target_polars.parse_type_string("Array(Int64, shape=(5,))") + assert result == pl.Array(pl.Int64, shape=(5,)) + + def test_parameterized_result_is_instance_not_class(self): + result = target_polars.parse_type_string( + "Datetime(time_unit='us', time_zone='UTC')" + ) + assert isinstance(result, pl.Datetime) + + +class TestBareNamesUnchanged: + def test_bare_name_still_resolves(self): + assert target_polars.parse_type_string("Int64") is pl.Int64 + assert target_polars.parse_type_string("String") is pl.String + + def test_bare_enum_categorical_still_resolve(self): + """Gap 4 resolution: the bare (no-param) Enum/Categorical forms + legitimately parse to the bare classes — unchanged, not newly invalid. + The empty-categories footgun of *using* them is pre-existing and + addressed via `categories`/parameterized strings, not by rejecting + the bare form here.""" + assert target_polars.parse_type_string("Enum") is pl.Enum + assert target_polars.parse_type_string("Categorical") is pl.Categorical + + +class TestGarbageRejected: + @pytest.mark.parametrize("s", [ + "garbage", + "Datetime(nonsense=1)", + "NotARealDtype", + "os.system('x')", + ]) + def test_unparseable_returns_none(self, s): + assert target_polars.parse_type_string(s) is None From 8b501529408c5df90e05542a0919863a699d2371 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:39:12 +1000 Subject: [PATCH 3/6] feat(dtypes): narwhals parameterized backend_type parsing (item 54, task 3) --- .../core/dtypes/target_narwhals.py | 19 ++++-- tests/core/dtypes/test_target_narwhals.py | 66 +++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 tests/core/dtypes/test_target_narwhals.py diff --git a/src/mountainash/core/dtypes/target_narwhals.py b/src/mountainash/core/dtypes/target_narwhals.py index 260ed579..fe03e9d5 100644 --- a/src/mountainash/core/dtypes/target_narwhals.py +++ b/src/mountainash/core/dtypes/target_narwhals.py @@ -50,9 +50,16 @@ def from_native(native: Any) -> Optional[D]: def parse_type_string(s: str) -> Optional[Any]: - if "(" in s: - return None - t = getattr(nw, s, None) - if t is not None and isinstance(t, type): - return t - return None + if "(" not in s: + t = getattr(nw, s, None) + return t if isinstance(t, type) else None # NOT issubclass(t, nw.DataType) — no such class + from ._paramstring import parse_constructor_repr + namespace = { + n: getattr(nw, n) for n in + ("Datetime", "Duration", "Decimal", "List", "Array", "Int8", "Int16", + "Int32", "Int64", "UInt8", "UInt16", "UInt32", "UInt64", "Float32", + "Float64", "Boolean", "String", "Binary", "Date", "Time", + "Categorical", "Enum") + } + result = parse_constructor_repr(s, namespace) + return result if isinstance(result, type) or result is not None else None diff --git a/tests/core/dtypes/test_target_narwhals.py b/tests/core/dtypes/test_target_narwhals.py new file mode 100644 index 00000000..ec16d00b --- /dev/null +++ b/tests/core/dtypes/test_target_narwhals.py @@ -0,0 +1,66 @@ +"""Narwhals parse_type_string — parameterized backend_type fidelity (item 54, gap 1). + +Mirrors the Polars cases against nw.*, plus the Important-finding-2 regression +guard: the post-upgrade guard must be `isinstance(t, type)`, NOT an +`issubclass(t, nw.DataType)` check — there is no `nw.DataType` class, so a +naive literal port of the Polars code would AttributeError at import time. +""" +from __future__ import annotations + +import narwhals as nw +import pytest + +from mountainash.core.dtypes import target_narwhals + + +class TestGuardAdaptedNotCopied: + def test_no_nw_datatype_class_exists(self): + """Premise of the Important-finding-2 guard: a literal port of the + Polars guard would reference a class that does not exist.""" + assert hasattr(nw, "DataType") is False + + def test_bare_name_guard_is_isinstance_type(self): + """DTypeClass instances (nw.Int64 etc.) ARE types; a passing + bare-name parse proves the guard was adapted, not copy-pasted.""" + assert target_narwhals.parse_type_string("Int64") is nw.Int64 + + +class TestParameterizedRoundTrip: + @pytest.mark.parametrize("s,expected", [ + ("Datetime(time_unit='us', time_zone='UTC')", + nw.Datetime(time_unit="us", time_zone="UTC")), + ("Duration(time_unit='ms')", nw.Duration(time_unit="ms")), + # NOTE: MountainashDtype has no canonical DECIMAL member + # (canonical.py) — backend_type is Decimal's ONLY path to a schema + # entry on every target; there is no canonical fallback to fall back + # to. That is why this test has no non-backend_type counterpart. + ("Decimal(precision=38, scale=10)", nw.Decimal(precision=38, scale=10)), + ("List(Int64)", nw.List(nw.Int64)), + ]) + def test_parameterized_repr_round_trips(self, s, expected): + assert target_narwhals.parse_type_string(s) == expected + + def test_array_with_bare_name_arg_and_shape_tuple(self): + """Critical-finding case (bare-Name arg + tuple-of-Constant kwarg).""" + result = target_narwhals.parse_type_string("Array(Int64, shape=(5,))") + assert result == nw.Array(nw.Int64, shape=(5,)) + + def test_enum_categories(self): + result = target_narwhals.parse_type_string("Enum(categories=['a', 'b'])") + assert result == nw.Enum(categories=["a", "b"]) + + +class TestBareNamesUnchanged: + def test_bare_names_still_resolve(self): + assert target_narwhals.parse_type_string("Int32") is nw.Int32 + assert target_narwhals.parse_type_string("String") is nw.String + + +class TestGarbageRejected: + @pytest.mark.parametrize("s", [ + "garbage", + "Datetime(nonsense=1)", + "NotARealDtype", + ]) + def test_unparseable_returns_none(self, s): + assert target_narwhals.parse_type_string(s) is None From fc2b77b5f4f9afc26af647d46e789979b0d709ba Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:39:49 +1000 Subject: [PATCH 4/6] feat(dtypes): pyarrow parameterized backend_type parsing (item 54, task 4) --- src/mountainash/core/dtypes/target_pyarrow.py | 26 +++++++++- tests/core/dtypes/test_target_pyarrow.py | 47 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 tests/core/dtypes/test_target_pyarrow.py diff --git a/src/mountainash/core/dtypes/target_pyarrow.py b/src/mountainash/core/dtypes/target_pyarrow.py index c331e0ba..da53ac74 100644 --- a/src/mountainash/core/dtypes/target_pyarrow.py +++ b/src/mountainash/core/dtypes/target_pyarrow.py @@ -2,6 +2,7 @@ """PyArrow target mappings. Imported lazily.""" from __future__ import annotations +import re from typing import Any, Optional import pyarrow as pa @@ -56,6 +57,27 @@ def from_native(native: Any) -> Optional[D]: def parse_type_string(s: str) -> Optional[Any]: try: - return pa.type_for_alias(s) + return pa.type_for_alias(s) # unparameterized names, unchanged except (KeyError, ValueError): - return None + pass + if m := _TIMESTAMP_RE.match(s): + unit, tz = m.groups() + return pa.timestamp(unit, tz=tz) if tz else pa.timestamp(unit) + if m := _DURATION_RE.match(s): + return pa.duration(m.group(1)) + if m := _TIME_RE.match(s): + bits, unit = m.groups() + return (pa.time32 if bits == "32" else pa.time64)(unit) + if m := _DECIMAL_RE.match(s): + bits, prec, scale = m.groups() + ctor = pa.decimal128 if bits == "128" else pa.decimal256 + return ctor(int(prec), int(scale)) + return None + + +# Bounded bracket/paren family — the only parameterized forms PyArrow's +# str() emits that type_for_alias does not already parse. +_TIMESTAMP_RE = re.compile(r"^timestamp\[(\w+)(?:, tz=(.+))?\]$") +_DURATION_RE = re.compile(r"^duration\[(\w+)\]$") +_TIME_RE = re.compile(r"^time(32|64)\[(\w+)\]$") +_DECIMAL_RE = re.compile(r"^decimal(128|256)\((\d+),\s*(\d+)\)$") diff --git a/tests/core/dtypes/test_target_pyarrow.py b/tests/core/dtypes/test_target_pyarrow.py new file mode 100644 index 00000000..8f9f6b46 --- /dev/null +++ b/tests/core/dtypes/test_target_pyarrow.py @@ -0,0 +1,47 @@ +"""PyArrow parse_type_string — parameterized backend_type fidelity (item 54, gap 1). + +PyArrow's str() format is bracket/paren grammar (timestamp[us, tz=UTC], +decimal128(38, 10)) — a distinct, bounded family parsed via regex after +pa.type_for_alias fails. Recursive list<...>/struct<...> strings are +explicitly out of scope and must stay None (never a silent partial parse). +""" +from __future__ import annotations + +import pyarrow as pa +import pytest + +from mountainash.core.dtypes import target_pyarrow + + +class TestParameterizedRoundTrip: + @pytest.mark.parametrize("s,expected", [ + ("timestamp[us, tz=UTC]", pa.timestamp("us", tz="UTC")), + ("timestamp[ns]", pa.timestamp("ns")), + ("decimal128(38, 10)", pa.decimal128(38, 10)), + ("decimal256(38, 10)", pa.decimal256(38, 10)), + ("duration[ms]", pa.duration("ms")), + ("time64[ns]", pa.time64("ns")), + ("time32[s]", pa.time32("s")), + ]) + def test_parameterized_repr_round_trips(self, s, expected): + assert target_pyarrow.parse_type_string(s) == expected + + def test_bare_name_via_alias_unchanged(self): + assert target_pyarrow.parse_type_string("int64") == pa.int64() + assert target_pyarrow.parse_type_string("string") == pa.string() + + +class TestOutOfScopeStaysNone: + @pytest.mark.parametrize("s", [ + "list", # recursive list grammar — out of scope + "struct", # recursive struct grammar — out of scope + "fixed_size_list[2]", + ]) + def test_recursive_grammar_returns_none(self, s): + """Asserted explicitly: a silent partial parse would be worse than + None (canonical fallback handles the container honestly).""" + assert target_pyarrow.parse_type_string(s) is None + + def test_garbage_returns_none(self): + assert target_pyarrow.parse_type_string("not_a_type") is None + assert target_pyarrow.parse_type_string("timestamp[us, tz=UTC") is None From 6fa1c3fb429e9ca7f36e259bf0f800a216b5f74f Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:40:25 +1000 Subject: [PATCH 5/6] test(dtypes): ibis/pandas parameterized parsing regression locks (item 54, task 5) --- .../dtypes/test_target_regression_locks.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/core/dtypes/test_target_regression_locks.py diff --git a/tests/core/dtypes/test_target_regression_locks.py b/tests/core/dtypes/test_target_regression_locks.py new file mode 100644 index 00000000..78109532 --- /dev/null +++ b/tests/core/dtypes/test_target_regression_locks.py @@ -0,0 +1,42 @@ +"""Regression locks for Ibis/Pandas parameterized backend_type parsing. + +These two targets were ALREADY correct at item 54's start (verified +empirically): ibis.dtype() is a full grammar parser, pandas_dtype() is the +official pandas parser. This item deliberately makes NO production change to +them — these tests guard against future upstream drift in libraries we don't +touch, exercised through the same registry.parse_type_string surface the +resolver uses. +""" +from __future__ import annotations + +import pytest + +from mountainash.core.dtypes import TypeTarget, registry + + +class TestIbisRegressionLocks: + @pytest.mark.parametrize("s", [ + "timestamp('UTC')", # parameterized temporal + "decimal(38, 9)", # parameterized decimal + "array", # parameterized list + ]) + def test_parameterized_strings_still_parse(self, s): + assert registry.parse_type_string(s, TypeTarget.IBIS) == s + + def test_garbage_still_rejected(self): + assert registry.parse_type_string("not a type", TypeTarget.IBIS) is None + + +class TestPandasRegressionLocks: + @pytest.mark.parametrize("s", [ + "datetime64[ns, UTC]", # parameterized temporal w/ tz + "datetime64[ns]", # plain temporal + "category", # categorical + "Int64", # pandas nullable int + "timedelta64[ns]", + ]) + def test_parameterized_strings_still_parse(self, s): + assert registry.parse_type_string(s, TypeTarget.PANDAS) == s + + def test_garbage_still_rejected(self): + assert registry.parse_type_string("not_a_dtype", TypeTarget.PANDAS) is None From cf98fed6342f62564728928d785ea0f138f7edb2 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Mon, 17 Aug 2026 13:45:45 +1000 Subject: [PATCH 6/6] feat(typespec): raise InvalidBackendTypeError on unparseable backend_type (item 54, task 6) --- src/mountainash/typespec/converters.py | 12 +++++- .../dag/test_resource_read_cross_backend.py | 21 ++++++---- tests/typespec/test_converters.py | 41 ++++++++++++++++++- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/mountainash/typespec/converters.py b/src/mountainash/typespec/converters.py index e4941699..40a66684 100644 --- a/src/mountainash/typespec/converters.py +++ b/src/mountainash/typespec/converters.py @@ -13,7 +13,12 @@ from typing import TYPE_CHECKING, Any, Dict -from mountainash.core.dtypes import MountainashDtype, TypeTarget, registry +from mountainash.core.dtypes import ( + InvalidBackendTypeError, + MountainashDtype, + TypeTarget, + registry, +) from mountainash.typespec.universal_types import to_canonical if TYPE_CHECKING: @@ -35,6 +40,11 @@ def _resolve_field_native(field: "FieldSpec", target: TypeTarget) -> Any: parsed = registry.parse_type_string(field.backend_type, target) if parsed is not None: return parsed + # Validation strictness (item 54, §5): only a non-empty, non-None + # backend_type that the target cannot parse raises. None/"" means "no + # override given" and falls through to canonical (item 53's ANY->STRING + # case relies on that). + raise InvalidBackendTypeError(field.name, field.backend_type, target) canon = to_canonical(field.type) if canon is None: # ANY canon = MountainashDtype.STRING diff --git a/tests/relations/dag/test_resource_read_cross_backend.py b/tests/relations/dag/test_resource_read_cross_backend.py index 01e10efa..f41cbf3a 100644 --- a/tests/relations/dag/test_resource_read_cross_backend.py +++ b/tests/relations/dag/test_resource_read_cross_backend.py @@ -517,14 +517,17 @@ def test_zero_row_column_completed(self, backend_ext): assert df.height == 0 -class TestInlineReadItem54Deferred: - """Assert the CURRENT (item-53) behaviour of dtypes deferred to item 54. - These are NOT the aspirational result — they lock what item 53 delivers so - item 54 can flip them deliberately. Do not 'fix' these here.""" - - def test_parameterized_backend_type_falls_to_string(self, backend_ext): - # tz-aware Datetime string contains '(' -> parse_type_string returns - # None -> ANY -> String. Item 54 will honour it. +class TestInlineReadParameterizedBackendType: + """Parameterized backend_type fidelity on the inline-read path (item 54). + + These flipped from item 53's deferred behaviour ("falls to String") in + PR-1 of item 54: a tz-aware Datetime string now parses to a real + parameterized dtype instead of degrading to ANY -> String.""" + + def test_parameterized_backend_type_produces_real_dtype(self, backend_ext): + # tz-aware Datetime string contains '(' — previously parse_type_string + # returned None -> ANY -> String; item 54 now reconstructs the real + # parameterized dtype. res = DataResource( name="t", format="json", data={"ts": [None]}, @@ -534,7 +537,7 @@ def test_parameterized_backend_type_falls_to_string(self, backend_ext): ]}, ) df = _collect_pl(backend_ext.read_resource(res)) - assert df.schema["ts"] == pl.String # item 54: should become tz-aware Datetime + assert df.schema["ts"] == pl.Datetime(time_zone="UTC") class TestInlineReadCastError: diff --git a/tests/typespec/test_converters.py b/tests/typespec/test_converters.py index 97850857..b422c359 100644 --- a/tests/typespec/test_converters.py +++ b/tests/typespec/test_converters.py @@ -234,6 +234,38 @@ def test_any_materializes_as_string(self): spec = TypeSpec(fields=[FieldSpec(name="a", type=UniversalType.ANY)]) assert to_polars_schema(spec)["a"] is pl.String + def test_invalid_backend_type_raises_on_every_fixed_target(self): + """Spec §7 test 6: the raise fires per-target on the three targets + whose parsers were upgraded (Polars/Narwhals/PyArrow). Ibis/Pandas + are skipped — their parsers are already correct and regression-locked + (Task 5), so an unparseable string there is not the primary surface.""" + from mountainash.core.dtypes import InvalidBackendTypeError, TypeTarget + from mountainash.typespec import TypeSpec, FieldSpec + from mountainash.typespec.universal_types import UniversalType + from mountainash.typespec.converters import _resolve_field_native + field = FieldSpec(name="x", type=UniversalType.INTEGER, backend_type="garbage") + for target in (TypeTarget.POLARS, TypeTarget.NARWHALS, TypeTarget.PYARROW): + with pytest.raises(InvalidBackendTypeError) as exc_info: + _resolve_field_native(field, target) + msg = str(exc_info.value) + assert "x" in msg # names the field + assert "garbage" in msg # names the string + assert target.value in msg # names its own target + + @pytest.mark.parametrize("backend_type", [None, ""]) + def test_empty_or_none_backend_type_falls_through(self, backend_type): + """Spec §5: backend_type=None/"" is 'no override given', not invalid + input — falls through to canonical (item 53's ANY->STRING case relies + on this).""" + import polars as pl + from mountainash.typespec import TypeSpec, FieldSpec + from mountainash.typespec.universal_types import UniversalType + from mountainash.typespec.converters import to_polars_schema + spec = TypeSpec(fields=[ + FieldSpec(name="x", type=UniversalType.INTEGER, backend_type=backend_type), + ]) + assert to_polars_schema(spec)["x"] is pl.Int64 + def test_backend_type_preferred_when_parseable(self): import polars as pl from mountainash.typespec import TypeSpec, FieldSpec @@ -244,12 +276,17 @@ def test_backend_type_preferred_when_parseable(self): ]) assert to_polars_schema(spec)["x"] is pl.Int32 - def test_unparseable_backend_type_falls_back(self): + def test_unparseable_backend_type_raises(self): + """Validation strictness (item 54, §5): a non-empty, non-None + backend_type that the target cannot parse raises — the resolver no + longer silently falls back to canonical.""" import polars as pl + from mountainash.core.dtypes import InvalidBackendTypeError from mountainash.typespec import TypeSpec, FieldSpec from mountainash.typespec.universal_types import UniversalType from mountainash.typespec.converters import to_polars_schema spec = TypeSpec(fields=[ FieldSpec(name="x", type=UniversalType.INTEGER, backend_type="garbage"), ]) - assert to_polars_schema(spec)["x"] is pl.Int64 + with pytest.raises(InvalidBackendTypeError, match="garbage"): + to_polars_schema(spec)