diff --git a/docs/reference/expression-coverage.json b/docs/reference/expression-coverage.json index 229b5123..3d9b2e75 100644 --- a/docs/reference/expression-coverage.json +++ b/docs/reference/expression-coverage.json @@ -70849,19 +70849,6 @@ "upstream_ref": null, "since": "2026-08-06" }, - { - "id": "NW-REL-03", - "kind": "engine_leniency", - "operation_keys": [], - "backends": [ - "narwhals-lazy" - ], - "summary": "narwhals-lazy has no sample() on a LazyFrame (AttributeError)", - "impact": "Relation.sample() raises on narwhals-lazy; eager backends sample rows", - "workaround": "Use an eager backend for sample()", - "upstream_ref": null, - "since": "2026-08-06" - }, { "id": "NW-STR-14", "kind": "semantics", diff --git a/docs/reference/expression-coverage.md b/docs/reference/expression-coverage.md index da4d27d8..63114fa4 100644 --- a/docs/reference/expression-coverage.md +++ b/docs/reference/expression-coverage.md @@ -578,7 +578,6 @@ Cells whose facts are all scoped (dialect / parameter / option / value-class) ha | NW-MATH-10 | engine_leniency | pandas, narwhals | `SIN`, `COS`, `TAN`, `ASIN`, `ACOS`, `ATAN`, `ATAN2`, `RADIANS`, `DEGREES`, `SINH`, `COSH`, `TANH`, `ASINH`, `ACOSH`, `ATANH` | pandas and narwhals lack native trigonometric, angular-conversion, and hyperbolic math functions; these ops raise NotImplementedError | trig (sin/cos/tan/asin/acos/atan/atan2), angular (radians/degrees), and hyperbolic (sinh/cosh/tanh/asinh/acosh/atanh) raise on pandas and all narwhals backends; polars and ibis (polars/duckdb) compute them | Use a polars or ibis-polars/ibis-duckdb backend for these math functions | NW-MATH-10 | 2026-08-06 | | NW-REL-01 | engine_leniency | narwhals-lazy | — | narwhals-lazy with_row_index() requires an explicit order_by= (row order over a LazyFrame is undefined); calling it without one raises TypeError | Relation.with_row_index() raises on narwhals-lazy; eager narwhals/polars and ibis-duckdb/ibis-sqlite assign a 0..N-1 index | Use an eager backend, or pass an explicit order before the lazy row index | — | 2026-08-06 | | NW-REL-02 | engine_leniency | narwhals | — | Narwhals does not support unnest of a struct column | Relation.unnest() raises on narwhals backends; polars and ibis compute it | Use a polars or ibis backend for unnest | — | 2026-08-06 | -| NW-REL-03 | engine_leniency | narwhals-lazy | — | narwhals-lazy has no sample() on a LazyFrame (AttributeError) | Relation.sample() raises on narwhals-lazy; eager backends sample rows | Use an eager backend for sample() | — | 2026-08-06 | | NW-STR-14 | semantics | narwhals-pandas | `TITLE`, `INITCAP` | narwhals-pandas title/initcap route to pandas str.title(); its Unicode titlecasing of sharp-S/ligatures differs from polars to_titlecase (e.g. 'ße' -> 'ẞe' vs 'SSe') | title()/initcap() on narwhals-pandas may differ from polars/narwhals-polars on non-ASCII inputs (sharp-S, ligatures); ASCII is identical | Use polars or narwhals-polars where exact polars titlecasing of non-ASCII is required | — | 2026-07-29 | | NW-STR-15 | semantics | pandas, narwhals | `LTRIM`, `RTRIM` | pandas and narwhals lack directional trimming: ltrim/rtrim and strip_chars_start/end strip BOTH sides (only strip_chars is native), so leading/trailing-only requests over-strip | ltrim/rtrim and str.strip_chars_start()/strip_chars_end() strip both sides on pandas/narwhals; polars and ibis strip only the requested side | Use a polars or ibis backend for directional trimming | — | 2026-08-06 | | NW-STR-17 | engine_leniency | pandas, narwhals | `REPEAT` | str.repeat(n) is unsupported on pandas and narwhals (BackendCapabilityError); no repeat translation is wired for these backends | ma.col(x).str.repeat(n) raises on pandas and all narwhals backends; polars and ibis compute it | Use a polars or ibis backend for str.repeat() | — | 2026-08-06 | diff --git a/src/mountainash/core/capabilities/divergences.py b/src/mountainash/core/capabilities/divergences.py index e6e53023..005f1175 100644 --- a/src/mountainash/core/capabilities/divergences.py +++ b/src/mountainash/core/capabilities/divergences.py @@ -647,17 +647,6 @@ def _all() -> tuple[DivergenceFact, ...]: upstream_ref=None, since="2026-08-06", ), - DivergenceFact( - id="NW-REL-03", - kind=DivergenceKind.ENGINE_LENIENCY, - operation_keys=(), # relation op sample - backends=("narwhals-lazy",), - summary="narwhals-lazy has no sample() on a LazyFrame (AttributeError)", - impact="Relation.sample() raises on narwhals-lazy; eager backends sample rows", - workaround="Use an eager backend for sample()", - upstream_ref=None, - since="2026-08-06", - ), DivergenceFact( id="IB-REL-10", kind=DivergenceKind.ENGINE_LENIENCY, diff --git a/src/mountainash/datacontracts/compiler.py b/src/mountainash/datacontracts/compiler.py index 15e6fc01..d5b6e363 100644 --- a/src/mountainash/datacontracts/compiler.py +++ b/src/mountainash/datacontracts/compiler.py @@ -109,16 +109,11 @@ def constraint_checks( ) ) if c is not None and c.pattern is not None: - # `pattern` is a regex (Frictionless pattern / regex-match semantics), - # NOT a literal substring — str.contains is Substrait literal contains, - # so a regex must go through regexp_match_substring (partial match -> - # not-null == the pattern matched somewhere). checks.append( RowRule( id=f"{col}__pattern", expr=_maybe_guard( - nullable, col, - ma.col(col).str.regexp_match_substring(c.pattern).is_not_null(), + nullable, col, ma.col(col).str.regex_contains(c.pattern) ), severity=severity, fields=[col], diff --git a/src/mountainash/datacontracts/validator.py b/src/mountainash/datacontracts/validator.py index a1fe433c..6ce1f0f2 100644 --- a/src/mountainash/datacontracts/validator.py +++ b/src/mountainash/datacontracts/validator.py @@ -6,6 +6,7 @@ from __future__ import annotations import inspect +import random from typing import TYPE_CHECKING, Any, Callable import polars as pl @@ -60,31 +61,38 @@ def _to_polars_frame(rel: Any) -> pl.DataFrame: if isinstance(materialised, pl.LazyFrame): materialised = materialised.collect() return materialised - @classmethod def _slice( - cls, rel: Any, *, head: int | None, tail: int | None, - sample: int | None, random_seed: int | None, - ) -> Any: - """17 P4: slicing happens exactly once, before the runner.""" - import mountainash as ma - + cls, + rel: Any, + *, + head: int | None, + tail: int | None, + sample: int | None, + random_seed: int | None, + ) -> tuple[Any, dict[str, Any]]: + """Apply slicing once, keeping seeded sampling on the source backend.""" + diagnostics: dict[str, Any] = {} if head is not None: rel = rel.head(head) if tail is not None: rel = rel.tail(tail) if sample is not None: - if random_seed is None: - # Relation.sample is cross-backend (no seed param; Ibis - # approximates n via fraction) — stays on the native backend. - rel = rel.sample(n=sample) + effective_seed = ( + random_seed if random_seed is not None else random.randrange(2**31) + ) + sampled = rel.sample(n=sample, seed=effective_seed) + if sampled.count_rows() == 0 and rel.count_rows() > 0: + rel = rel.limit(sample) + diagnostics["sample_fallback"] = { + "reason": "sampled slice was empty on non-empty input", + "requested_sample": sample, + "random_seed": effective_seed, + "fallback": f"limit({sample})", + } else: - # Seeded sampling: Relation.sample has no seed parameter, so - # deterministic sampling materialises to Polars (documented - # narrowing; follow-on backlog: seed option on Relation.sample). - frame = cls._to_polars_frame(rel) - rel = ma.relation(frame.sample(n=sample, seed=random_seed)) - return rel + rel = sampled + return rel, diagnostics # -- public API ----------------------------------------------------------- @@ -175,7 +183,9 @@ def _run( # --- data phase prepared = self._prepare_data(data, context) rel = prepared if isinstance(prepared, Relation) else ma.relation(prepared) - rel = self._slice(rel, head=head, tail=tail, sample=sample, random_seed=random_seed) + rel, slice_diagnostics = self._slice( + rel, head=head, tail=tail, sample=sample, random_seed=random_seed + ) if getattr(self.contract.Config, "coerce", True): rel = rel.conform(spec) @@ -191,6 +201,8 @@ def _run( validator_name=self.name, datacontract_name=self.contract.contract_name(), ) + if slice_diagnostics: + result.diagnostics.update(slice_diagnostics) if skipped: # skipped summaries are visibility only: appended to the frame, # never part of the runner's pass computation (they cannot fail) diff --git a/src/mountainash/exceptions.py b/src/mountainash/exceptions.py index be92e203..e9e497ec 100644 --- a/src/mountainash/exceptions.py +++ b/src/mountainash/exceptions.py @@ -21,6 +21,7 @@ ConformTransformError, SchemaDriftError, ) +from mountainash.relations.core.errors import InvalidSampleArgumentsError from mountainash.relations.dag.errors import ( DAGError, RelationDAGRequired, @@ -53,6 +54,7 @@ __all__ = [ "MountainashError", "InvalidOptionValueError", + "InvalidSampleArgumentsError", "BareExpressionCollectionError", "ConformError", "MissingFieldsError", diff --git a/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py b/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py index 82e90322..dc2b5ffb 100644 --- a/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py +++ b/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py @@ -45,14 +45,20 @@ def explode(self, relation: ir.Table, /, *, columns: list[str]) -> ir.Table: return result def sample( - self, relation: ir.Table, /, *, n: Optional[int] = None, fraction: Optional[float] = None + self, + relation: ir.Table, + /, + *, + n: Optional[int] = None, + fraction: Optional[float] = None, + seed: Optional[int] = None, ) -> ir.Table: if fraction is not None: - return relation.sample(fraction) + return relation.sample(fraction, method="row", seed=seed) if n is not None: total = relation.count().execute() frac = min(n / total, 1.0) if total > 0 else 1.0 - return relation.sample(frac) + return relation.sample(frac, method="row", seed=seed) raise ValueError("Either n or fraction must be specified for sample().") def unpivot( diff --git a/src/mountainash/relations/backends/relation_systems/narwhals/extensions_mountainash/relsys_nw_ext_ma_util.py b/src/mountainash/relations/backends/relation_systems/narwhals/extensions_mountainash/relsys_nw_ext_ma_util.py index 00727a3c..ee3717cf 100644 --- a/src/mountainash/relations/backends/relation_systems/narwhals/extensions_mountainash/relsys_nw_ext_ma_util.py +++ b/src/mountainash/relations/backends/relation_systems/narwhals/extensions_mountainash/relsys_nw_ext_ma_util.py @@ -52,8 +52,16 @@ def sample( *, n: Optional[int] = None, fraction: Optional[float] = None, + seed: Optional[int] = None, ) -> Any: - return relation.sample(n=n, fraction=fraction) + frame = relation + is_lazy = isinstance(frame, nw.LazyFrame) + if is_lazy: + frame = frame.collect() + if n is not None: + n = min(n, len(frame)) + sampled = frame.sample(n=n, fraction=fraction, seed=seed) + return sampled.lazy() if is_lazy else sampled def unpivot( self, diff --git a/src/mountainash/relations/backends/relation_systems/polars/extensions_mountainash/relsys_pl_ext_ma_util.py b/src/mountainash/relations/backends/relation_systems/polars/extensions_mountainash/relsys_pl_ext_ma_util.py index 97ff585f..31cbb68c 100644 --- a/src/mountainash/relations/backends/relation_systems/polars/extensions_mountainash/relsys_pl_ext_ma_util.py +++ b/src/mountainash/relations/backends/relation_systems/polars/extensions_mountainash/relsys_pl_ext_ma_util.py @@ -78,9 +78,13 @@ def sample( *, n: Optional[int] = None, fraction: Optional[float] = None, + seed: Optional[int] = None, ) -> pl.LazyFrame: # LazyFrame does not support .sample() directly — collect, sample, re-lazy. - return relation.collect().sample(n=n, fraction=fraction).lazy() + frame = relation.collect() + if n is not None: + n = min(n, frame.height) + return frame.sample(n=n, fraction=fraction, seed=seed).lazy() def unpivot( self, diff --git a/src/mountainash/relations/core/errors.py b/src/mountainash/relations/core/errors.py index e7e8d149..da0c2eec 100644 --- a/src/mountainash/relations/core/errors.py +++ b/src/mountainash/relations/core/errors.py @@ -16,3 +16,7 @@ def __init__(self, node_type: type) -> None: f"via RelationVisitRegistry.register({node_type.__name__}, handler) " f"or define _operation_key plus a RelationOperationDef for it." ) + + +class InvalidSampleArgumentsError(MountainashError, ValueError): + """Relation.sample() argument-contract violation.""" diff --git a/src/mountainash/relations/core/relation_api/relation.py b/src/mountainash/relations/core/relation_api/relation.py index 0caead4b..8e759cc7 100644 --- a/src/mountainash/relations/core/relation_api/relation.py +++ b/src/mountainash/relations/core/relation_api/relation.py @@ -407,13 +407,29 @@ def sample( *, n: Optional[int] = None, fraction: Optional[float] = None, + seed: Optional[int] = None, ) -> Relation: - """Sample rows.""" + """Sample rows using a validated common argument contract.""" + from mountainash.relations.core.errors import InvalidSampleArgumentsError + + if (n is None) == (fraction is None): + raise InvalidSampleArgumentsError( + "sample() requires exactly one of n or fraction " + f"(got n={n!r}, fraction={fraction!r})" + ) + if n is not None and n < 0: + raise InvalidSampleArgumentsError(f"sample(n=...) must be >= 0, got {n}") + if fraction is not None and not (0.0 <= fraction <= 1.0): + raise InvalidSampleArgumentsError( + f"sample(fraction=...) must be in [0, 1], got {fraction}" + ) options: dict[str, Any] = {} if n is not None: options["n"] = n if fraction is not None: options["fraction"] = fraction + if seed is not None: + options["seed"] = seed return self._make( ExtensionRelNode( input=self._node, diff --git a/src/mountainash/relations/core/relation_protocols/prtcl_relation_api.py b/src/mountainash/relations/core/relation_protocols/prtcl_relation_api.py index c5291cff..59bbb749 100644 --- a/src/mountainash/relations/core/relation_protocols/prtcl_relation_api.py +++ b/src/mountainash/relations/core/relation_protocols/prtcl_relation_api.py @@ -83,7 +83,13 @@ def drop_nans(self, *, subset: Optional[list[str]] = None) -> Self: ... def with_row_index(self, *, name: str = "index") -> Self: ... def explode(self, *columns: Any) -> Self: ... def unnest(self, *columns: str, separator: str) -> Self: ... - def sample(self, *, n: Optional[int] = None, fraction: Optional[float] = None) -> Self: ... + def sample( + self, + *, + n: Optional[int] = None, + fraction: Optional[float] = None, + seed: Optional[int] = None, + ) -> Self: ... def unpivot( self, diff --git a/src/mountainash/relations/core/relation_protocols/relation_systems/extensions_mountainash/prtcl_relsys_ext_ma_util.py b/src/mountainash/relations/core/relation_protocols/relation_systems/extensions_mountainash/prtcl_relsys_ext_ma_util.py index 5ceb4dab..a3a8144a 100644 --- a/src/mountainash/relations/core/relation_protocols/relation_systems/extensions_mountainash/prtcl_relsys_ext_ma_util.py +++ b/src/mountainash/relations/core/relation_protocols/relation_systems/extensions_mountainash/prtcl_relsys_ext_ma_util.py @@ -19,7 +19,13 @@ def with_row_index(self, relation: RelationT, /, *, name: str = "index") -> Rela def explode(self, relation: RelationT, /, *, columns: list[str]) -> RelationT: ... def sample( - self, relation: RelationT, /, *, n: Optional[int] = None, fraction: Optional[float] = None + self, + relation: RelationT, + /, + *, + n: Optional[int] = None, + fraction: Optional[float] = None, + seed: Optional[int] = None, ) -> RelationT: ... def unpivot( diff --git a/tests/_spine_expectation_census.md b/tests/_spine_expectation_census.md index addec6fe..9879c312 100644 --- a/tests/_spine_expectation_census.md +++ b/tests/_spine_expectation_census.md @@ -146,7 +146,6 @@ Buckets: `migrated` (derivable from the spine today), `retained` (a LITERAL_ONLY | tests/relations/cross_backend/test_rel_extension_ops_results.py:39 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-REL-01') — migrated | | tests/relations/cross_backend/test_rel_extension_ops_results.py:40 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-REL-02') — migrated | | tests/relations/cross_backend/test_rel_extension_ops_results.py:41 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('MA-REL-01') — migrated | -| tests/relations/cross_backend/test_rel_extension_ops_results.py:42 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-REL-03') — migrated | | tests/relations/cross_backend/test_rel_join_results.py:42 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-REL-11') — migrated | | tests/relations/cross_backend/test_terminal_scalar_aggregates.py:13 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-AGG-05') — migrated | | tests/relations/cross_backend/test_terminal_scalar_aggregates.py:14 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-AGG-03') — migrated | diff --git a/tests/datacontracts/test_compiler.py b/tests/datacontracts/test_compiler.py index 91c25191..0c5ebc0b 100644 --- a/tests/datacontracts/test_compiler.py +++ b/tests/datacontracts/test_compiler.py @@ -1,6 +1,7 @@ """Tests for contract_from_typespec — TypeSpec to native BaseDataContract.""" from __future__ import annotations +import pytest import polars as pl from mountainash.typespec.spec import TypeSpec, FieldSpec, FieldConstraints @@ -208,3 +209,42 @@ def test_no_constraints_produces_nullable_field(self): df = pl.DataFrame({"val": [None, "x"]}) result = Contract.validate_datacontract(df) assert result.passes is True + +class TestPatternCheckCrossBackend: + """Pattern checks must fail on non-matching values on every backend.""" + + def _spec(self): + return _make_spec( + FieldSpec( + name="code", + type=UniversalType.STRING, + constraints=FieldConstraints(pattern=r"^[a-z]{3}-[0-9]{2}$"), + ) + ) + + def _data(self, backend, values): + df = pl.DataFrame({"code": values}) + if backend == "narwhals-pandas": + import narwhals as nw + + return nw.from_native(df.to_pandas()) + if backend == "ibis-duckdb": + ibis = pytest.importorskip("ibis") + + return ibis.duckdb.connect().create_table("t", df.to_arrow()) + return df + + @pytest.mark.parametrize("backend", ["polars", "narwhals-pandas", "ibis-duckdb"]) + def test_non_matching_value_fails(self, backend): + result = self._spec().to_contract(name="pattern_xb").validate_datacontract( + self._data(backend, ["abc-12", "###"]) + ) + assert not result.passes + assert "code__pattern" in _failing_check_ids(result) + + @pytest.mark.parametrize("backend", ["polars", "narwhals-pandas", "ibis-duckdb"]) + def test_matching_values_pass(self, backend): + result = self._spec().to_contract(name="pattern_xb").validate_datacontract( + self._data(backend, ["abc-12", "xyz-99"]) + ) + assert result.passes diff --git a/tests/datacontracts/test_validator.py b/tests/datacontracts/test_validator.py index ba073c6c..867aff0d 100644 --- a/tests/datacontracts/test_validator.py +++ b/tests/datacontracts/test_validator.py @@ -8,7 +8,7 @@ from mountainash.datacontracts.validator import Validator from mountainash.datacontracts.contract import BaseDataContract from mountainash.datacontracts.field import Field -from mountainash.datacontracts.rule import Rule, guarded +from mountainash.datacontracts.rule import Rule from mountainash.datacontracts.registry import RuleRegistry from mountainash.datacontracts.result import ValidationResult @@ -176,3 +176,79 @@ class SimpleContract(BaseDataContract): result = validator.validate(df, head=2) assert result.processor is not None assert len(result.processor._source_data) == 2 + +class TestSeededSlice: + """Seeded validation slices stay on backend and avoid vacuous passes.""" + + def _contract(self): + from mountainash.typespec.spec import FieldConstraints, FieldSpec, TypeSpec + from mountainash.typespec.universal_types import parse_universal + + spec = TypeSpec(fields=[ + FieldSpec( + name="a", + type=parse_universal("integer"), + constraints=FieldConstraints(minimum=0), + ), + ]) + return spec.to_contract(name="seeded_slice") + + def test_empty_sample_slice_falls_back_and_records_diagnostics(self, monkeypatch): + from mountainash.relations import Relation + + def empty_sample(self, *, n=None, fraction=None, seed=None): + return self.head(0) + + monkeypatch.setattr(Relation, "sample", empty_sample) + df = pl.DataFrame({"a": list(range(20))}) + result = self._contract().validate_datacontract(df, sample=5, random_seed=1) + assert "sample_fallback" in result.diagnostics + assert result.diagnostics["sample_fallback"]["requested_sample"] == 5 + totals = [ + summary["total_rows"] + for summary in result.check_summaries.to_dicts() + if summary["total_rows"] is not None + ] + assert totals and all(total == 5 for total in totals) + + def test_implicit_fallback_records_effective_seed(self, monkeypatch): + from mountainash.datacontracts import validator as validator_module + from mountainash.relations import Relation + + monkeypatch.setattr(validator_module.random, "randrange", lambda _: 12345) + monkeypatch.setattr(Relation, "sample", lambda self, **_: self.head(0)) + result = self._contract().validate_datacontract( + pl.DataFrame({"a": list(range(20))}), sample=5 + ) + assert result.diagnostics["sample_fallback"]["random_seed"] == 12345 + + @pytest.mark.parametrize("backend", ["polars", "pandas", "ibis-duckdb"]) + def test_seeded_validate_is_deterministic(self, backend): + df = pl.DataFrame({"a": [i - 10 for i in range(100)]}) + if backend == "pandas": + data = df.to_pandas() + elif backend == "ibis-duckdb": + ibis = pytest.importorskip("ibis") + data = ibis.duckdb.connect().create_table("t", df.to_arrow()) + else: + data = df + contract = self._contract() + first = contract.validate_datacontract(data, sample=20, random_seed=7) + second = contract.validate_datacontract(data, sample=20, random_seed=7) + assert first.passes == second.passes + assert first.check_summaries.drop("elapsed").to_dicts() == second.check_summaries.drop("elapsed").to_dicts() + assert first.failure_cases.to_dicts() == second.failure_cases.to_dicts() + + def test_never_validates_empty_slice_silently_on_ibis(self): + ibis = pytest.importorskip("ibis") + con = ibis.duckdb.connect() + table = con.create_table("t", pl.DataFrame({"a": list(range(30))}).to_arrow()) + contract = self._contract() + for seed in range(8): + result = contract.validate_datacontract(table, sample=1, random_seed=seed) + totals = [ + summary["total_rows"] + for summary in result.check_summaries.to_dicts() + if summary["total_rows"] is not None + ] + assert any(total > 0 for total in totals) or "sample_fallback" in result.diagnostics diff --git a/tests/relations/cross_backend/test_rel_extension_ops_results.py b/tests/relations/cross_backend/test_rel_extension_ops_results.py index 7b338365..7688b313 100644 --- a/tests/relations/cross_backend/test_rel_extension_ops_results.py +++ b/tests/relations/cross_backend/test_rel_extension_ops_results.py @@ -39,7 +39,6 @@ _WRI = [pytest.param(b, marks=xfail_divergence("NW-REL-01", backend=b)) for b in ALL_BACKENDS] _UNNEST = [pytest.param(b, marks=xfail_divergence("NW-REL-02", backend=b)) for b in STRUCT_BACKENDS] _PIVOT = [pytest.param(b, marks=xfail_divergence("MA-REL-01", backend=b)) for b in ALL_BACKENDS] -_SAMPLE = [pytest.param(b, marks=xfail_divergence("NW-REL-03", backend=b)) for b in ALL_BACKENDS] def sorted_dicts(dicts: list[dict], by: str | list[str]) -> list[dict]: @@ -319,19 +318,61 @@ def test_top_k_by_column(self, backend_name, backend_factory): @pytest.mark.cross_backend -@pytest.mark.parametrize("backend_name", _SAMPLE) +@pytest.mark.parametrize("backend_name", ALL_BACKENDS) class TestSample: - def test_sample_n(self, backend_name, backend_factory): - df = backend_factory.create( - {"a": list(range(20))}, backend_name + """Seeded sampling is deterministic within each backend.""" + + def _frame(self, backend_factory, backend_name): + return backend_factory.create( + {"a": list(range(20)), "b": [i * 10 for i in range(20)]}, + backend_name, ) - result = ma.relation(df).sample(n=5).to_dicts() + + def _mark_seed_support(self, request, backend_name): + reasons = { + "ibis-polars": "ibis-polars: Table.sample with a random seed is unsupported", + "ibis-sqlite": "ibis-sqlite: Table.sample with a random seed is unsupported", + } + if backend_name in reasons: + request.node.add_marker(pytest.mark.xfail(strict=True, reason=reasons[backend_name])) + + def test_sample_n_row_count(self, backend_name, backend_factory, request): + self._mark_seed_support(request, backend_name) + df = self._frame(backend_factory, backend_name) + result = ma.relation(df).sample(n=5, seed=7).to_dicts() if backend_name.startswith("ibis-"): - # Ibis converts n to a fraction (Bernoulli per-row), so the sample size has no - # guaranteed floor — it can legitimately return anywhere from 0 to len(input) rows. assert 0 <= len(result) <= 20 else: assert len(result) == 5 - all_values = set(range(20)) - for row in result: - assert row["a"] in all_values + + def test_same_seed_same_rows(self, backend_name, backend_factory, request): + self._mark_seed_support(request, backend_name) + df = self._frame(backend_factory, backend_name) + first = ma.relation(df).sample(n=5, seed=42).to_dicts() + second = ma.relation(df).sample(n=5, seed=42).to_dicts() + assert sorted_dicts(first, "a") == sorted_dicts(second, "a") + + def test_same_seed_same_rows_fraction(self, backend_name, backend_factory, request): + self._mark_seed_support(request, backend_name) + df = self._frame(backend_factory, backend_name) + first = ma.relation(df).sample(fraction=0.4, seed=3).to_dicts() + second = ma.relation(df).sample(fraction=0.4, seed=3).to_dicts() + assert sorted_dicts(first, "a") == sorted_dicts(second, "a") + + def test_oversize_n_returns_all_rows(self, backend_name, backend_factory, request): + df = self._frame(backend_factory, backend_name) + result = ma.relation(df).sample(n=100, seed=1).to_dicts() + assert sorted_dicts(result, "a") == sorted_dicts( + ma.relation(df).to_dicts(), "a" + ) + + def test_fraction_zero_returns_no_rows(self, backend_name, backend_factory, request): + df = self._frame(backend_factory, backend_name) + assert ma.relation(df).sample(fraction=0.0, seed=1).to_dicts() == [] + + def test_fraction_one_returns_all_rows(self, backend_name, backend_factory, request): + df = self._frame(backend_factory, backend_name) + result = ma.relation(df).sample(fraction=1.0, seed=1).to_dicts() + assert sorted_dicts(result, "a") == sorted_dicts( + ma.relation(df).to_dicts(), "a" + ) diff --git a/tests/relations/test_rel_api_build.py b/tests/relations/test_rel_api_build.py index 20fc6053..a400966d 100644 --- a/tests/relations/test_rel_api_build.py +++ b/tests/relations/test_rel_api_build.py @@ -396,3 +396,53 @@ def test_raises_on_bare_read(self): node = ReadRelNode(dataframe="x") leaf = Relation._find_leaf_read_node(node) assert leaf.dataframe == "x" + + +class TestSampleArgumentContract: + """Relation.sample validates its common argument contract at build time.""" + + def _rel(self) -> Relation: + return relation("df") + + def test_neither_n_nor_fraction_raises(self): + from mountainash.exceptions import InvalidSampleArgumentsError + + with pytest.raises(InvalidSampleArgumentsError, match="exactly one"): + self._rel().sample() + + def test_both_n_and_fraction_raises(self): + from mountainash.exceptions import InvalidSampleArgumentsError + + with pytest.raises(InvalidSampleArgumentsError, match="exactly one"): + self._rel().sample(n=2, fraction=0.5) + + def test_negative_n_raises(self): + from mountainash.exceptions import InvalidSampleArgumentsError + + with pytest.raises(InvalidSampleArgumentsError, match=">= 0"): + self._rel().sample(n=-1) + + def test_fraction_out_of_range_raises(self): + from mountainash.exceptions import InvalidSampleArgumentsError + + for bad in (-0.1, 1.5): + with pytest.raises(InvalidSampleArgumentsError, match=r"\[0, 1\]"): + self._rel().sample(fraction=bad) + + def test_error_is_valueerror_compat(self): + from mountainash.core.errors import MountainashError + from mountainash.exceptions import InvalidSampleArgumentsError + + assert issubclass(InvalidSampleArgumentsError, ValueError) + assert issubclass(InvalidSampleArgumentsError, MountainashError) + + def test_seed_lands_in_options(self): + assert self._rel().sample(n=2, seed=42)._node.options == {"n": 2, "seed": 42} + + def test_seed_omitted_from_options_when_none(self): + assert self._rel().sample(fraction=0.5)._node.options == {"fraction": 0.5} + + def test_boundary_arguments_are_valid(self): + assert self._rel().sample(n=0)._node.options == {"n": 0} + assert self._rel().sample(fraction=0.0)._node.options == {"fraction": 0.0} + assert self._rel().sample(fraction=1.0)._node.options == {"fraction": 1.0} \ No newline at end of file