[FEATURE](pyspark) PySpark validation generated from the Pydantic schema#518
[FEATURE](pyspark) PySpark validation generated from the Pydantic schema#518Seth Fitzsimmons (sethfitz) wants to merge 14 commits into
Conversation
GitHub Actions
Inactive
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
b23c092 to
038a1aa
Compare
GitHub Actions
Inactive
038a1aa to
36d9a39
Compare
GitHub Actions
Inactive
36d9a39 to
0bdd012
Compare
GitHub Actions
Inactive
0bdd012 to
1da7d9e
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
This seems reasonable for as a basis for review, but for merging, I don't see the upside in committing 15-30 generated files and 15K-30K generated lines to source control—we want to source control the generator, not the generatee, right? |
|
Divergence from Python no. 1 comment.
Basically we're saying PySpark will slightly under-count duplicates in some rare edge cases. While annoying, this doesn't seem like it's a very big deal. |
|
Divergence from Pydantic no. 3 question:
Does this mean we have stricter constraints in the PySpark than in Pydantic? If yes, shouldn't they be the same? |
|
Regarding
This is true in terms of struct-typed columns, but isn't it not true for a top-level dataframe? i.e. Doesn't it mean that if I have a model like: @no_extra_fields
class Foo(BaseModel):
bar: intAnd a dataframe like:
Wouldn't the |
|
Divergence from Pydantic no. 2 comment:
Let's see if I understand this right. I have a model like this: @require_any_of("bar", "baz")
class Foo(BaseModel):
bar: int | None = None
baz: str | None = NoneI think you're saying that Pydantic would allow The latest version of the Pydantic should also reject it... The docs say:
That seems to be the current behavior: >>> from pydantic import BaseModel
>>> from overture.schema.system.model_constraint import require_any_of
>>> @require_any_of("bar", "baz")
... class Foo(BaseModel):
... bar: int | None = None
... baz: str | None = None
...
>>> Foo(bar=None)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/ANT.AMAZON.COM/schapper/overture/schema/.venv/lib/python3.10/site-packages/pydantic/main.py", line 250, in __init__
validated_self = self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 1 validation error for Foo
Value error, at least one of these fields must be set to a value other than None, but none are: bar, baz (`@require_any_of`) [type=value_error, input_value={'bar': None}, input_type=dict]
For further information visit https://errors.pydantic.dev/2.12/v/value_errorAm I missing something, or did you actually achieve perfect parity? |
Victor Schappert (vcschapp)
left a comment
Victor Schappert (vcschapp)
left a comment
There was a problem hiding this comment.
Nice work on this.
It's a huge PR, but I think the successfulness of it is best captured by this comment:
Every constraint Pydantic enforces today is dispatched to a PySpark expression, which allows Spark to push down predicates and take advantage of Catalyst to avoid unnecessary deserialization ...
That's phenomenal.
A few points to discuss, with my biggest items being committing of generated code; and some minor conceptual issues around the use of the word "feature".
We agreed in one of the Schema WG meetings to drop the generated code from the PR to merge (once we've reviewed it), as it can be regenerated on demand and will exist in published packages for posterity.
Exactly. Agreed.
For
No, these would be rejected when checking the Spark schema (before moving into per-row validation).
I did achieve parity with Pydantic, but wanted to call this out because of the ability to omit values there (and when using JSON). |
GitHub Actions
Inactive
44393a5 to
1c21815
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
pytest-testmon tracks which tests cover which source files and skips unaffected tests on subsequent runs. Activated via a TESTMON Makefile variable so the default `make check` uses incremental selection while `make check TESTMON=` runs the full suite. Lock the dependency in the dev group, gitignore the local cache file, and thread $(TESTMON) through the test, test-all, and test-only targets. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Pull the shared `dimension` and `comparison` fields of the five vehicle selector subtypes into a `VehicleSelectorBase` parent, and thread `discriminator="dimension"` through the `VehicleSelector` annotated union. The discriminator turns the union into a Pydantic discriminated union, so it serializes as JSON Schema's `oneOf` + `discriminator` rather than `anyOf`. Regenerated segment_baseline_schema.json captures the new shape. This is a prerequisite for downstream tooling that walks discriminated unions structurally (e.g. PySpark codegen for segment's nested vehicle scoping). Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Replace the Tonga-based Division/DivisionArea/DivisionBoundary fixtures with Kauaʻi County samples that exercise admin_level, capital_division_ids, wikidata, and source license alongside the existing fields. Replace the Tonga-based Connector/Segment fixtures with a Vermooten Street junction in Pretoria that exercises access_restrictions with when.vehicle, speed_limits with when.heading, routes with ref, road_surface, and multi-source attribution. Reformat the TOML with 4-space indents and sorted keys to match sibling theme packages. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Introduce overture-schema-pyspark, a runtime PySpark validation
package whose per-feature expression modules and conformance tests
are generated from the same Pydantic models that define the schema,
along with an `overture-validate` CLI.
Runtime (overture-schema-pyspark/src/overture/schema/pyspark/):
- check.py — Check, CheckShape, FeatureValidation dataclasses.
- schema_check.py — write-first comparison of Spark schemas against
an expected StructType, with structural type matching and
SchemaMismatch reporting.
- validate.py — public API: validate_feature(), evaluate_checks(),
explain_errors(). The explain stage UNPIVOTs per-row check results
into one row per violation, preserving all input columns for
downstream join-back.
- cli.py — `overture-validate <parquet-or-directory>` runs the
validation pipeline against a path of GeoParquet files. Output is
one row per violation: feature ID, theme/type, failing field,
check name, offending value. Single-pass evaluation keeps memory
bounded for arbitrarily large inputs.
- expressions/ — shared runtime utilities (constraint_expressions,
column_patterns, _schema_structs). Per-feature expression modules
live under expressions/overture/ and are added by the codegen in
a follow-up commit.
- tests/_support/ — conformance test infrastructure (scenarios,
harness, helpers, mutations). The harness builds one DataFrame
per feature, applies all scenarios as deterministic-UUID-tagged
rows, runs validation once, and indexes violations back to
scenario IDs — O(checks) rather than O(checks * scenarios).
CLI filtering options:
--theme <theme> limit to one theme
--feature <feature> limit to one feature type
--skip-schema-check run only constraint checks (no schema
comparison)
--count-only print violation counts per check rather
than rows
--suppress <key> suppress specific (feature, field, check)
triples per a YAML config
Codegen pipeline (overture-schema-codegen/src/.../pyspark/):
FeatureSpec
|
constraint_dispatch.py map constraints to descriptors
|
check_builder.py walk FieldSpec -> CheckNode IR;
resolve array nesting, variant gating
|
schema_builder.py FieldSpec -> SchemaField list
(StructType source)
|
renderer.py CheckNode -> per-feature expression
module
test_renderer.py CheckNode -> per-feature conformance
test module
synthetic.py FeatureSpec -> BASE_ROW + invalid values
|
pipeline.py orchestrate, return GeneratedModule list
The dispatch tables map every supported constraint (Ge/Gt/Le/Lt/
Interval, MinLen/MaxLen, StrippedConstraint, PatternConstraint,
UniqueItemsConstraint, GeometryTypeConstraint, JsonPointerConstraint,
RequireAnyOfConstraint, RadioGroupConstraint, RequireIfConstraint,
ForbidIfConstraint, MinFieldsSetConstraint), NewType (Country-
CodeAlpha2, LinearlyReferencedRange, RegionCode), and base type
(HttpUrl, EmailStr) to constraint_expressions check functions.
Discriminated unions (segment is the canonical hard case) split
into per-arm test files. The codegen handles arm splitting via
generate_arm_rows in synthetic.py and _filter_field_nodes_for_arm
in test_renderer.py.
The Makefile gains a `generate-pyspark` target and gates `check`
on it so a stale generation surfaces immediately. The CLI is exposed
as a `[project.scripts]` entry point so `overture-validate`
becomes available after `pip install` / `uv sync`.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Generate PySpark expressions (and tests) for models defined in the workspace Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
PySpark 3.4 (the declared floor) doesn't run on Java 21, the default JDK on ubuntu-latest runners -- it hits NoSuchMethodException on java.nio.DirectByteBuffer.<init>(long, int), removed in JDK 21. Pin the lowest-direct cell to Java 17 so the resolved pyspark==3.4.0 can actually start. The default cell (which resolves to a current pyspark 4.x) keeps the runner's default Java 21. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
validate_feature built check expressions referencing every column the schema declares, then evaluated them with an eager df.select. When the input DataFrame lacked a declared column, Spark's plan analysis raised an AnalysisException before the caller could inspect the schema mismatch, so a file missing a required column produced a Java stack trace instead of the schema-mismatch report the CLI is built to emit. Columns that compare_schemas reports as absent from the data now have their checks dropped, the same as --skip-columns columns; referencing them is what crashes Spark. The mismatch is still recorded in schema_mismatches, so the CLI reports it and exits cleanly (or, with --skip-schema-check, validates the columns that are present). The CLI also prints the --skip-columns invocation for the absent columns, so the escape hatch is discoverable from the error itself. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Model-level constraints (require_any_of and the like) generated for a sub-model reached through an optional field fired even when that field was null. Pydantic skips a model validator when the optional sub-model is absent, so the generated PySpark expression produced a false positive the schema itself never raises. ModelCheck now carries a gate: the optional-ancestor path that must be non-null for the constraint to apply. check_builder sets it when the constrained model is reached via an optional struct field inside an array; the renderer wraps the constraint in F.when(<accessor>.isNotNull(), ...). Regenerated Segment expressions: the speed_limits[].when, access_restrictions[].when, and prohibited_transitions[].when require_any_of checks are now skipped when their when sub-model is null. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
This lands map key/value validation in the PySpark and Markdown generators, renames the codegen's core "feature" vocabulary to "model", and closes correctness gaps across validation generation, constraint identity, base-row synthesis, and the runtime checks. Map key/value validation (PySpark). Dict-typed fields such as names.common (dict[LanguageTag, StrippedString], present on nearly every feature with names) got no key or value validation, so invalid language-tag keys and unstripped values passed overture-validate while failing Pydantic. A MapPath IR locates a map's keys or values, the check builder descends into MapOf.key/.value, and the renderer emits map_keys_check/map_values_check over F.map_keys/F.map_values, reusing the null-guarded array transform. A map value that is a sub-model is descended into the way a list element is, so its field and model-level constraints generate checks. Shapes with no representable MapPath -- a container nested in a map value, an array-shaped map projection, a map reached through an array -- raise at generation time rather than silently drop a constraint or emit a mis-typed check. Map key/value rendering (Markdown). Map references render every key and value type and their directly-applied constraints. Map sides link to their type pages, container wrappers (a list or map inside a map) survive instead of being peeled, model-valued map values link, and every variant is named rather than collapsing to map<K, ?>. A bare side folds into the surrounding map<...> code span so two adjacent backtick spans cannot corrupt the CommonMark. feature to model vocabulary. The codegen generates validation for any Pydantic BaseModel, not only geospatial features, so the root abstraction is now ModelSpec (a RecordSpec, or a tagged UnionSpec of records) and the runtime surface is validate_model, model_keys, model_names, ModelValidation, and the emitted MODEL_VALIDATION constant, in place of validate_feature, feature_keys, feature_names, FeatureValidation, and FEATURE_VALIDATION. The registry walks the new constant name and the module template is renamed to match. The overture-validate CLI argument and the GeoJSON-feature validator keep feature vocabulary, where the geospatial meaning is correct. Graceful degradation over absent and skipped columns. Each Check carries one read_columns set, the top-level columns it actually reads, replacing the split root_field/referenced_fields mechanism that left an unresolvable F.col() for a row-root constraint over a struct field and for an in-array constraint branching on a row-root discriminator. Checks whose columns are skipped or structurally absent -- top-level, nested (sources[].confidence resolves to sources), or model-level -- are dropped before Spark instead of crashing with a raw AnalysisException. The CLI backstop classifies the exception through the structured PySpark error API rather than its message text, so a real planning bug propagates as a traceback instead of steering the operator to suppress it with --skip-columns. ValidationResult exposes absent_columns, the same derivation that drives check dropping. evaluate_checks and explain_errors reject input columns colliding with their reserved _err_N, field, check, and message names. Field-check label disambiguation. Discriminated unions produced multiple field checks sharing a (field, name) identity -- segment emitted two required checks on access_restrictions[].when.vehicle[].value, and sources[].confidence collided on bounds across many models -- leaving colliding rows indistinguishable in suppression, explain_errors metadata, and the conformance expected_field. Symmetric _N suffixes disambiguate them, computed over the unfiltered check list so per-arm test filtering cannot hide a collision the shared expression module still carries. Label and disambiguated function-name derivation is unified into one flattening shared by the expression and test renderers, which also fixes a per-arm asymmetry where a model-check base label spanning an arm boundary made the per-arm test expect a field the module never emits. Base-row and test-data value synthesis. Row synthesis handles Not(FieldEqCondition), used by Division and DivisionBoundary as ~IS_COUNTRY, instead of silently treating it as unsatisfied, and merges every bound constraint on a field before choosing a value so both-exclusive intervals and sibling Gt/Lt constraints are jointly satisfied. forbid_if fill values are typed for non-string scalars (0, 0.0, False) rather than falling back to a string sentinel for a non-string column. One CONSTRAINT_VALUES table pairs the valid and invalid value for each string constraint, replacing two parallel tables that drifted silently. SparkCategory dispatch is exhaustive: an unhandled or binary category raises at generation time instead of emitting a wrong fill value. Raw Field(pattern=) handling fails loud on an uncurated pattern naming the table to update, and requires PydanticMetadata before reading an object's .pattern, restoring the unhandled-constraint TypeError contract. Constraint identity. Deduplication compares constraints by value. System FieldConstraint subclasses inherited object identity, so two structurally identical UniqueItemsConstraint or PatternConstraint instances reported as divergent for any union field shared across members. FieldConstraint now defines value equality and hashing keyed on the concrete type and normalized instance state, reducing a compiled re.Pattern to (pattern, flags) so a case-insensitive pattern is not masked and reducing container attributes to hashable forms, so a set of constraints deduplicates by rule. The union fingerprint compares the constraint objects directly, falling back to a value-stable repr only for pydantic's internal Field() metadata, the lone constraint type that still compares by identity. Constraint attributes must themselves be value types, a contract the base states for future authors. Runtime checks. check_geometry_type reads the full four-byte WKB type word and normalizes ISO and EWKB encodings to the OGC base type, so 3D (Z/M/ZM) geometries in GeoParquet no longer false-fail every geometry-type check. check_url_format matches the scheme case-insensitively, matching Pydantic's HttpUrl. check_bounds rejects NaN under a lower-only bound, which Spark's ordering otherwise lets pass. resolve_read completes a partial partition path, appending a type=Y leaf below a theme=X/ path so one feature's checks no longer run against every type sharing a theme directory. A compiled, flagged re.Pattern, the only Pydantic carrier for re.IGNORECASE, is honored in both the pyspark check and the markdown display instead of crashing the model's generation. Variant gating and extraction. A discriminated union reached under a struct prefix, and a gate crossing mismatched array nesting, raise rather than emit a mis-gated check; both are preemptive, since no current schema reaches that state. Forward-ref and self-referential field annotations resolve against their owning model before classification rather than crashing the terminal classifier on an unresolved string. A required list[X | None] field no longer inherits is_optional from element nullability. Build, structure, and docs. The check-python-code CI paths trigger on the Makefile and the workflow themselves, uv-sync no longer routes its errors to /dev/null, and test-all runs the full suite unconditionally so golden-JSON and example-only changes are not deselected by testmon. typing-extensions is declared to match its unconditional import. Shared helpers replace duplicated logic -- register_model, schema_const_name, enum_source, a struct-only-prefix predicate, and a model-spec discovery entry point that gives every discovery site one extraction carrying partition layout -- and several docstrings and CLI help strings are corrected to match current behavior. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Consolidate duplicated decision logic behind single arbiters and close correctness gaps across the PySpark codegen and runtime. Runtime: - check_geometry_type flags a WKB blob too short to hold a full type word as a violation, gating on hex length (a valid header is 5 bytes: a 1-byte order flag plus a 4-byte type word). A length gate, not a null test, is required: conv() returns NULL only for a 0-1 byte blob, while a 2-4 byte blob parses a truncated header into a non-null bogus type (b"\x01\x01" reads as the Point code) that would otherwise pass. - check_bounds skips the NaN guard for integer columns via a check_nan flag. An integer column cannot be NaN, so the guard was dead work; this drops two casts and an isnan call at 54 integer bound sites across 16 generated files. Codegen correctness (latent on current schemas): - analyze_type preserves a list element's description when the element carries the field's only prose. - A model constraint on a dict[K, Model] value generates a test that mutates the map value rather than the row root, so the invalid row trips the violation it claims to test. Codegen consolidation (generated output unchanged): - Derive Check/ModelCheck read_columns from the IR instead of a regex over rendered source. Each FieldPath, Guard, and constraint variant names its column sources structurally and raises on any unhandled variant, replacing the regex's silent incompleteness. A test cross-checks the IR-derived columns against the rendered source across every real model, guarding the renderer/IR coupling the regex could not desync from. - Route every map-shape decision through classify_map_projection, the single arbiter of representable map projections. - Route map-side and NewType underlying-type linking through _scalar_identity, the single linkable-identity predicate. - Share primitive fill values through PRIMITIVE_FILL_TABLE across the three SparkCategory consumers. - Extract _top_level for dotted-name collapsing and _reject_struct_only_prefix for the struct-nested guards. Imports: - Hoist function-local imports to module top level project-wide and enforce it through ruff PLC0415. The lone deliberate cycle-breaker (extract_union in model_extraction) keeps a documented noqa. Also folds in deferred review nits: a stronger bounds-kwargs test, split map-projection rejection messages, and test-module reordering. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Generated validation carried two latent runtime bugs. error_msg built
its message with F.concat, which yields NULL when any interpolated value
is NULL — array_compact then dropped that element along with the
violation, so an out-of-bounds value read as valid whenever an
interpolated value was itself NULL (a linear-reference range like [null,
1.5]). Each value now coalesces to a string before concatenation.
Separately, _peel_union dropped every Literal arm wherever a concrete
arm coexisted, so generated checks rejected model-valid literals — the
empty string on annex *_url fields and "Global" on countries. The
dropped literals now survive as a LiteralAlternatives constraint, and a
runtime except_literals wrapper lets those exact values pass the
concrete arm's content checks while a null still fails check_required.
The markdown reference gains an "Also accepts" note.
Both bugs were invisible to the conformance suite, which passed without
exercising the check target. The per-scenario ::valid row was a plain
base-row copy, so for the ~77% of scenarios whose target is reachable
only through scaffolded nesting it asserted nothing. The valid row now
merges the scaffold onto the base row with no mutation, carrying a
constraint-satisfying value at the target. The scaffold generator builds
every container on the path as a valid base row, descends a
discriminated-union element into its seeded arm, and fills single-level
arrays for min_length and uniqueness. coerce_to_schema casts each
numeric to its declared column type before createDataFrame so a value
scaffolded for a narrowed union arm stays valid. A forbid_if whose
condition the base row triggers forbids its own target field, so the
scaffold now flips that condition field to a value the forbid rejects
and re-runs constraint satisfaction. An unreachable scaffold path raises
at generation time instead of emitting a target-absent {}, and an
unbuildable scenario fails with its id rather than routing to
pytest.skip.
Codegen needed correctness and ordering fixes. Restore UnionSpec's
@DataClass(eq=False) — the rewrite dropped it, giving the spec
value-equality over mutable field lists and leaving it unhashable where
consumers key on object identity. schema_builder raises on a zero-field
StructType and on union fields that share a name but resolve to
differing non-widening Spark types. Output is now deterministic:
reverse_references dedups referrers in insertion order, and
_find_common_base breaks max-MRO-depth ties on module and qualname. The
check field and name render through py_literal so a name carrying a
quote or backslash stays valid Python, the float NaN guard derives from
primitive_spark_category, and pipeline picks the no-arm test filename by
arm is not None so a falsy discriminator cannot collide with it.
A verify-pyspark-generated make target and a CI git diff --exit-code now
gate the committed generated tree — make check regenerates it before
tests, so stale output was overwritten and never verified.
register_model teardown uses REGISTRY.pop instead of del, so a test that
already dropped the key no longer raises KeyError inside finally and
masks the body's own exception.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Generate a PySpark validator and conformance tests for the `require_any_true` model constraint (#546), so divisions' `is_land` / `is_territorial` rule is enforced in Spark, not just Pydantic and Markdown. Support is positive-boolean-`FieldEqCondition`-only, enforced at `RequireAnyTrue.__post_init__` (and independently in base-row synthesis, which reads the raw constraint). The runtime coalesces a null condition to False, which matches Python's `None == value` only for a positive equality; a negated or non-boolean condition would emit a wrong Spark check, so it fails loudly at codegen instead. Pipeline stages: - constraint_dispatch: a self-validating `RequireAnyTrue` descriptor. The `FieldEq` unwrap helpers (`parse_field_eq` / `require_field_eq`) live here, the lowest layer that needs them. - runtime `check_require_any_true`: a disjunction over the conditions with each null coalesced to False. - renderer: lower each condition through the `require_if` path; boolean values render as `F.lit(True)` so ruff's E712 leaves the Column comparison intact -- a latent gap that also affected boolean `require_if` / `forbid_if` conditions. - base_row: satisfy one condition so the valid baseline passes Pydantic. - test renderer: an "all conditions false" mutation. Regenerate the PySpark expressions and conformance tests. The diff spans every theme because the rebase onto post-#546 main also picked up the provider/resource/version string-typing change, which adds min_length and snake_case checks to the shared Sources model. DivisionArea's e2e test moves from radio_group to require_any_true to match the changed source. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
6396ee2 to
ceeb31a
Compare
Given how close we are to merging, it seems reasonable to update the PR to drop the generated code now. |
IMO we should update the PR to provide exact parity to Pydantic. There's nothing useful served by having diverging The |
overture-schema-codegen and overture-schema-common shipped with no requires-python at all, so their published metadata advertised support for every Python version. pip would resolve and install them on 3.9 and below, where they fail at import: both packages depend on overture-schema-system, which declares >=3.10 and uses 3.10-only syntax. The floor existed in practice and went unstated in the one place installers read. Both now declare >=3.10, matching the root workspace and the other eleven packages. Placement follows each file's local key ordering -- last in the alphabetized codegen table, between description and license in common, where its sibling overture-schema-system puts it. uv.lock is unchanged: >=3.10 is what uv already resolved against. Raising the floor to 3.11 is deliberately not part of this change; it waits on Python 3.10's end of life (2026-10-31). Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
|
I found a conceptual bug of omission that's minor and I don't think it should block, but I do think we should note it in a GH issue for later fixing. It's more of a general code generation issue rather than anything specific to PySpark. IssueWe "support" generating code for certain collection types that are supported by Pydantic, however the generated code is wrong both for Markdown and PySpark. The important ones are:
The bonus one would be fixed-length tuples where the underlying type Spark type of all the elements is the same base type, as in:
What happens now?Here's how the system behaves today with the various kinds of inputs:
Why isn't this important to fix now?
|
Victor Schappert (vcschapp)
left a comment
Victor Schappert (vcschapp)
left a comment
There was a problem hiding this comment.
I've been over the PR as best as I can given the significant scope.
Same conclusion as before, I think it's ready to merge and any minor tweaks or good ideas in the comments can be done after the fact.
Two exceptions, mentioned early in the comment stream 👆:
- We should drop the generated code before merging.
- We should reduce
BBoxvalidation to parity with Pydantic for now and offload advance validation to a separate issue.
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class ScalarPath: |
There was a problem hiding this comment.
Given this can address either a scalar value or a whole struct (I think, correct if wrong) I wonder if a better name here might be SinglePath or DirectPath.
I don't feel strongly about this, just an idea.
There was a problem hiding this comment.
There's a real imprecision here, but I think it's tangled with the map-nesting thread, so I've pulled both into #570 rather than rename now. The sharper version of the point: ArrayPath / MapPath name how the path fans out, while ScalarPath names what's at the end — a mixed axis. But we can't rename the members cleanly before settling the taxonomy: if a path can ever traverse both an array and a map (the maps thread below), ArrayPath vs MapPath stops being a partition and the split collapses toward Direct (no iteration) vs Iterated. So the rename rides on that call.
Drafted by Claude with Seth's oversight.
| projection: MapProjection | ||
|
|
||
|
|
||
| PathSegment: TypeAlias = StructSegment | ArraySegment |
There was a problem hiding this comment.
I find this type name a bit challenging because the exclusion of MapSegment feels arbitrary, hard to explain, in something with such a general name as PathSegment.
It's not heavily used, suggest dropping the explicit type alias and just inserting StructSegment | ArraySegment everywhere.
I don't feel strongly about this, just an idea.
There was a problem hiding this comment.
Agreed — the name claimed a generality it didn't have, sitting right next to FieldSegment (the actual Struct | Array | Map one). Only two use sites, so I've inlined StructSegment | ArraySegment and dropped the alias. #571.
Drafted by Claude with Seth's oversight.
| and report grouping), not how to access the data. The expression in | ||
| `expr` already encodes the access pattern. | ||
|
|
||
| `read_columns` names every top-level schema column the expression |
There was a problem hiding this comment.
Would be helpful to expand on the relationship between the singular expr: Column and the plural read_columns: frozenset[str].
There was a problem hiding this comment.
Expanded the docstring to spell out the relationship: expr and read_columns are two views of one computation — expr is the single composed Column, read_columns is its read-set (every top-level column that one expression dereferences). They're carried separately because the builder knows the columns as it composes expr, and recording them beats recovering them from the finished Column. #571.
Drafted by Claude with Seth's oversight.
| - `MapPath` -- struct segments leading to a map column, a single | ||
| `MapSegment` projecting the map to its keys or values, then a struct-only | ||
| leaf (possibly empty). Locates a value reached by iterating a | ||
| `dict[K, V]`'s keys or values, encoded with a `{key}` / `{value}` marker | ||
| on the map column and the leaf appended after it (e.g. `names.common{key}` | ||
| for a scalar value, `subs{value}.label` for a field inside a | ||
| `dict[K, Model]` value). |
There was a problem hiding this comment.
The limitations on where maps can appear seem arbitrary.
It feels like the tail (limitation of the stringized path representation grammar) might be wagging the dog (of allowed representations).
In both Pydantic land and Spark land, map values can be scalars, arrays, structs, or maps; and that structure can continue deep into the schema. So this doesn't feel like a limitation based on trying to fit to the lowest common denominator, it feels more like a limitation that's either arbitrary, or caused by a limitation in the path grammar, or (unnecessarily) reflecting some other known limitation elsewhere in the program.
I have a feeling that with refactoring, this module could end up being both more generalized (supporting the structured children of map use cases) with potentially fewer lines of code...
I don't at all consider this a blocker to merging. It's a useful discussion to have but any fixing could easily be documented as a GH issue and done as a follow-up.
There was a problem hiding this comment.
Filed as #570. Checked the premise while I was in there: the only map in the schema today is CommonNames (dict[LanguageTag, StrippedString]), so nothing currently reaches the guards — they're latent gap-markers, not live bugs, which is what makes deferring safe. The asymmetry is the interesting part: MapPath already carries a dict[K, Model] leaf case nothing in the schema uses, while hard-blocking dict[K, list] and nested maps the other way. So the real fork is generalize-fully (one iterated path type, segments mixing array + map) vs narrow-to-dict[K, scalar]. #570 carries that plus the ScalarPath naming, since they turn out to be the same decision.
Drafted by Claude with Seth's oversight.
…ranch This commit's diff is the substantive difference between this branch (the current #518 state) and the rewritten pyspark-expression-codegen branch, computed on a common base so it excludes the unrelated origin/main advance: - generated PySpark expression and conformance-test trees dropped from git and gitignored; the drift-check Make target and CI step removed - BBox validation reduced to Pydantic parity (latitude ordering/range checks pulled and stashed on #531) - README release examples refreshed to 2026-06-17.0; walkthrough wording tightened Not for merge -- it exists so reviewers of the new PR can see the delta from the reviewed #518 branch at a glance. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
|
Closed in favor of #569 |
2 participants
Summary
Adds a new runtime package (
overture-schema-pyspark) plus a new output target inoverture-schema-codegenthat emits PySpark validation expressions and conformance tests from the same Pydantic models that define the schema. Ships anoverture-validateCLI (to be merged with theoverture-schemaCLI later; that will be an opportunity to improve its ergonomics) for running the validation against Parquet on disk or in S3.PySpark plugs in as a peer of the existing Markdown output target: same
ModelSpecextraction, same four-layer architecture (Discovery -> Extraction -> Output Layout -> Rendering), new pipeline module. The codegen abstraction is model-general (ModelSpec = RecordSpec | UnionSpec: one record that compiles to a SparkStructType, or a tagged union of records); in this schema every model is an Overture feature, so the generated modules are organized by theme/feature. Seepackages/overture-schema-codegen/docs/design.mdfor the full picture; the "PySpark Pipeline" section there covers the new stages in detail.What's in the PR
packages/overture-schema-pyspark/-- runtime. Public API invalidate.py(validate_model,explain_errors), schema comparison inschema_check.py, dataclasses incheck.py, theoverture-validateCLI incli.py, and shared expression building blocks inexpressions/{constraint_expressions,column_patterns,_schema_structs}.py. The per-feature expression modules underexpressions/generated/overture/schema/<theme>/<feature>.pyand per-feature conformance tests undertests/generated/overture/schema/<theme>/test_<feature>.pyare emitted by codegen and confined to agenerated/boundary thatmake generate-pysparkwipes and recreates._registry.pywalks that tree at import time and exposesREGISTRY: dict[str, ModelValidation]keyed by each module'sENTRY_POINTvalue (e.g."overture.schema.transportation:Segment"), reading each module's emittedMODEL_VALIDATIONconstant, plus a parallelPARTITION_MAPderived from each module'sPARTITIONSdict for partitioned features.Validation degrades gracefully when columns are skipped (
--skip-columns/--suppress) or structurally absent from the data. Every check declares the top-level columns it reads, and a check over a missing column -- top-level, nested inside an array or map (sources[].confidenceresolves tosources), or model-level -- is dropped before Spark planning rather than raising a rawAnalysisException.ValidationResult.absent_columnssurfaces the columns dropped for this reason, and the CLI backstop classifies any residual planning error through the structured PySpark error API so a generator bug propagates as a traceback instead of being mistaken for a missing column.This has been tested to work with Spark versions 3.4.0 - 4.1.1 (and the
lowest-directCI check will verify that it continues to work with the lowest declared PySpark version, which is currently3.4).Adam Lastowka (@Rachmanin0xFF) the public API changed since the previous version you tested (the entry point is now
validate_model, returning aValidationResult, in an attempt to simplify how it gets integrated with the CDP). If you point me to its current state, I can propose an update that uses the new API.packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/-- new output target. Pipeline stages:make generate-pysparkwipes bothgenerated/trees and recreates them;make checkgates on regeneration being current.What's covered
Every constraint Pydantic enforces today is dispatched to a PySpark expression, which allows Spark to push down predicates and take advantage of Catalyst to avoid unnecessary deserialization:
Ge/Gt/Le/Lt/Interval,ArrayMinLen/ArrayMaxLen/ScalarMinLen/ScalarMaxLen(typed length constraints split by attachment layer),StrippedConstraint,PatternConstraint,UniqueItemsConstraint,GeometryTypeConstraint,JsonPointerConstraint.names.common, adict[LanguageTag, StrippedString]present on most named features) have their per-key and per-value constraints validated by projectingmap_keys/map_valuesand applying the same field checks. A map value that is a sub-model is descended into, validating its field and model-level constraints.dict[K, Any]maps (e.g.Infrastructure.source_tags) carry no constraint and emit no checks; container-valued map values (a list or map inside a map value) are not yet supported and raise at generation time rather than emit an unanchored check.LinearlyReferencedRange(length / bounds / order).HttpUrl(format + length),EmailStr,BBox(completeness, lat ordering, lat range).RequireAnyOfConstraint,RadioGroupConstraint,RequireIfConstraint,ForbidIfConstraint,MinFieldsSetConstraint.MinFieldsSetConstraintmirrors Pydantic'smodel_fields_setsemantics: required fields are always set (the constructor enforces them) and count toward the threshold alongside any explicitly-set optional fields.NoExtraFieldsConstraintis intentionally skipped, as its behavior is implicit when using DataFrames.Known semantic gaps / differences
Divergences from Pydantic:
UniqueItemsConstraintuses Spark'sarray_distinct, which compares whole elements with structural equality on raw stored values. Pydantic compares normalized Python objects -- e.g.,list[HttpUrl]is compared after URL normalization (such as ensuring that URLs likehttp://example.comcontain a trailing/:http://example.com/). The PySpark check catches exact duplicates, not duplicates after normalization.require_any_ofchecksisNotNullas a proxy for Pydantic'smodel_fields_set. Parquet has no equivalent of "explicitly provided" because of the tabular nature of DataFrames;isNotNullis stricter (it rejects fields explicitly set to null).BBox's PySpark expressions include checking latitude ordering and range. Longitude is skipped due to undefined behavior around how we handle features that cross the antimeridian.PatternConstraintis evaluated with Java's regex dialect rather than Python's -- the two agree for the schema's patterns, but ASCII-vs-Unicode shorthand classes (\w,\d) and interior-newline handling differ in general.CLI
Output is one row per violation: feature ID, theme/type, failing field, check name, message, offending value.
Testing
overture-schema-codegengenerates conformance tests alongside the PySpark expressions as a sanity-check by evaluating expected-good and expected-bad rows within a single DataFrame (setup/teardown overhead for DataFrame-per-test-case was much too high). Each feature gets two baseline rows --BASE_ROW_SPARSE(required fields only) andBASE_ROW_POPULATED(every optional field filled in) -- and every scenario is exercised against both, so checks that depend on optional structure being present are covered as well as the minimal-row case.make checkgenerates tests and runs the full suite over everything, which use PySpark and increase test runtime (again; they run in just over a minute for me).testmonwas introduced to improve the developer experience by skipping tests that weren't affected by recent edits.Beyond the unit and conformance tests:
Notes for review
pyspark/{constraint_dispatch,check_builder,check_ir,schema_builder,renderer,test_renderer,pipeline}.pyplus the path IR inoverture-schema-system'sfield_path.pyand the runtime inoverture-schema-pyspark/src/overture/schema/pyspark/. Everything undergenerated/is regenerable output -- review the codegen, but skim the output to understand the shape of what's produced.VehicleSelectorBaseextraction, Java 17 CI pin, build/CI hardening sotest-allruns unconditionally and the check job triggers on the Makefile/workflow themselves,typing-extensionsdeclared) rather than splitting them into separate PRs.Closes #517.