Reparent string constraints under PatternConstraint + test improvements#458
Open
Seth Fitzsimmons (mojodna) wants to merge 4 commits intodevfrom
Open
Reparent string constraints under PatternConstraint + test improvements#458Seth Fitzsimmons (mojodna) wants to merge 4 commits intodevfrom
Seth Fitzsimmons (mojodna) wants to merge 4 commits intodevfrom
Conversation
Victor Schappert (vcschapp)
requested changes
Mar 4, 2026
Collaborator
Victor Schappert (vcschapp)
left a comment
There was a problem hiding this comment.
This is fantastic, especially the:
Net -112 lines.
And also the:
Enables isinstance-based dispatch for pattern constraints in downstream codegen.
Would you mind humoring me with the numpy-style docs? I was so close to getting it all on numpy.
packages/overture-schema-system/src/overture/schema/system/field_constraint/string.py
Outdated
Show resolved
Hide resolved
Eliminates duplicated validate() and __get_pydantic_json_schema__() across CountryCodeAlpha2, HexColor, LanguageTag, NoWhitespace, SnakeCase, PhoneNumber, RegionCode, and WikidataId constraints. Each is now a thin __init__-only wrapper calling super().__init__(). PatternConstraint gains optional keyword-only description, min_length, max_length parameters for JSON Schema annotations. StringConstraint gains _raise_validation_error() to deduplicate error construction across PatternConstraint, JsonPointerConstraint, and StrippedConstraint.
The previous pattern ^(\S.*)?\S$ required at least one non-whitespace
character, rejecting empty string. The validator itself accepts empty
string ("" == "".strip()), so the JSON schema was more restrictive
than the Python validation.
New pattern ^(\S(.*\S)?)?$ matches empty string (outer group optional),
single non-whitespace chars, and strings bookended by non-whitespace.
Updated all JSON schema baselines and inline expectations.
Replace `len(errors()) > 0` with error message assertions in hex color and no-whitespace invalid tests. The weak assertions only checked that validation failed, not that the correct error was raised.
Replace 16 individual valid/invalid test methods with two parametrized tests driven by PATTERN_CONSTRAINT_CASES. Covers all 8 PatternConstraint subclasses: LanguageTag, CountryCodeAlpha2, RegionCode, WikidataId, PhoneNumber, HexColor, NoWhitespace, SnakeCase. Moved SnakeCaseConstraint tests from TestErrorHandling (where they were misplaced) into the parametrized data. Non-PatternConstraint tests remain as standalone methods: base PatternConstraint (custom pattern), StrippedConstraint, and JsonPointerConstraint (empty-string special case).
aa3a6c3 to
2b8a8bd
Compare
Collaborator
Author
|
Now -285 net lines after I found some more things to simplify. I also fixed a JSON Schema validation bug with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
StringConstraintsubclasses (CountryCodeAlpha2,HexColor,LanguageTag,NoWhitespace,SnakeCase,PhoneNumber,RegionCode,WikidataId) underPatternConstraintas thin__init__-only wrappersPatternConstraintwith optionaldescription,min_length,max_lengthkeyword parameters for JSON Schema annotations_raise_validation_error()helper onStringConstraintto deduplicate error construction acrossPatternConstraint,JsonPointerConstraint, andStrippedConstraintStrippedConstraintJSON schema pattern to accept empty string (matching Python validator behavior)-285 net lines across constraints and tests. Enables
isinstance-based dispatch for pattern constraints in downstream codegen.JsonPointerConstraintandStrippedConstraintremain as directStringConstraintsubclasses (non-regex validation logic).Test plan
isinstanceparametrized test confirms all 8 subclasses arePatternConstraintinstancesPatternConstraintkwargs emit/omit JSON Schema annotations correctlyStrippedConstraintempty-string acceptance tested against both validator and JSON schema patternmake checkpasses