Pydantic package organization episode 1: "Making huge mess" - #397
Merged
Conversation
Victor Schappert (vcschapp)
requested a review
from Seth Fitzsimmons (sethfitz)
October 3, 2025 00:17
Victor Schappert (vcschapp)
temporarily deployed
to
staging
October 3, 2025 00:17 — with
GitHub Actions
Inactive
Victor Schappert (vcschapp)
temporarily deployed
to
staging
October 3, 2025 00:19 — with
GitHub Actions
Inactive
Seth Fitzsimmons (sethfitz)
requested changes
Oct 3, 2025
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Oct 3, 2025
#397 (comment) The proximate issue was that BSD `sed` on MacOS doesn't support alternation unless you use `-E`. But also the alternation wasn't needed, so I removed it.
Victor Schappert (vcschapp)
temporarily deployed
to
staging
October 3, 2025 20:26 — with
GitHub Actions
Inactive
Victor Schappert (vcschapp)
force-pushed
the
pydantic-packaging
branch
from
October 3, 2025 21:01
a912f1a to
6bc4c71
Compare
Victor Schappert (vcschapp)
temporarily deployed
to
staging
October 3, 2025 21:01 — with
GitHub Actions
Inactive
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Oct 3, 2025
#397 (comment) The proximate issue was that BSD `sed` on MacOS doesn't support alternation unless you use `-E`. But also the alternation wasn't needed, so I removed it.
This commit used to move "sources" out of the way for now, but that's over now after rebasing on Seth's #394.
…and TrimmedString -> StrippedString (stripped = Python term, not trim)
…yet... ...Too many broken docs to bother
#397 (comment) The proximate issue was that BSD `sed` on MacOS doesn't support alternation unless you use `-E`. But also the alternation wasn't needed, so I removed it.
Victor Schappert (vcschapp)
force-pushed
the
pydantic-packaging
branch
from
October 3, 2025 21:21
6bc4c71 to
f202c57
Compare
Victor Schappert (vcschapp)
temporarily deployed
to
staging
October 3, 2025 21:21 — with
GitHub Actions
Inactive
Seth Fitzsimmons (sethfitz)
approved these changes
Oct 3, 2025
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Oct 9, 2025
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Oct 9, 2025
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Oct 13, 2025
Re-org. Pydantic packages (2/N) The destination is as described in PR #397 (1/N) and PR #400. In this commit, the following value is added: 1. 🐞 Fix bug: The `@any_of` decorator was not working with transporation destinations. The bug was discovered while refactoring the `mixin` module and the fix is reflected in the updated transportation schema baseline. 2. 🐞 Fix bug: The `make doctest` target exited with code 0 (success) even if it found doctest failures. This bug was added in #397 when the `doctest` target was added and is now fixed. 3. Apply multiple suggestions from PR #397 including renaming `CountryCode` to `CountryCodeAlpha2` and dropping an unnecessary attribute from `PatternConstraint`. 4. Rename the sources package to annex in alignment as shown in the destination package diagram. 5. Begin refactoring `mixin` module to fix some known issues and polish it for final release. Changes made here: - Underlying implementation is replaced with the Pydantic function `create_model` (as was done in the scoping POC) which eliminates the need for `ConstrainedBaseModel` and should also eliminate MRO issues. - Underlying implementation uses some basic "JSON Schema algebra" to ensure that multiple decorators can coexist without breaking each others' JSON Schema changes. - The module name `mixin` is replaced by `model_constraint`, which feels more use case-oriented. - Initially one main decorator is migrated from `mixin` in the validation package to `model_constraint` in the system package. The rest will come next PR. 6. Migrate much of the validation package `README.md` into the system package's main module `__init__.py` and migrate the examples into doctests. 7. Re-home a few other bits of functionality from validation to core. 8. Bring the https://github.com/vcschapp/overture-schema-pydantic scoping into core as `scoping.py` and fix up the linter errors. This is not wired up yet, but that's Coming Soon, likely in episode 4. 9. At Seth's suggestion, switch `ConfidenceScore` from `float64` to `float32` though we will have to double-check the actual value in the Parquet model here. Stay tuned for episode 3, where we will finish re-homing the validation package.
Seth Fitzsimmons (sethfitz)
pushed a commit
that referenced
this pull request
Oct 19, 2025
The destination is as described in PR #397 (1/N), PR #400, and PR #403. In this commit, the following value is added: 1. 🐞 Fix bug: The `not_required_if` mixin wasn't meeting the original schema intent, which result in the `DivisionBoundary` feature type having the wrong JSON Schema. The expected behavior is "if the boundary is at the country level, then the `country` pair should be omitted" but instead it did two variations: in the Python validator, it required the country pair to be present; and in the JSON Schema, it required the country pair to be present, but only if the condition was not true. Both were subtly wrong. The bug is fixed in this commit and the mixin is renamed to `forbid_if`, which I hope will make the semantics more obvious. 2. 🐞 Fix bug: Very subtle, but the mixin model constraint validators did not work correctly if added directly to a `Feature` model, because they aren't aware of the quirky multi-tiered GeoJSON JSON Schema where most properties get pushed down into "properties", but some do not. This is (hopefully ... 🤞) fixed in the new `Feature` model in the system package. 3. 🐞 Fix bug: Very subtle, but the previous `Feature` model's JSON Schema did not prohibit fields named `"id"`, `"bbox"`, and `"geometry"` within the properties object. This created a weird quirk where in a poisoned JSON object containing those values under properties, the properties version would overwrite the root version. This is fixed by explicitly prohibiting those fields in the JSON Schema for the `Feature` class' properties object. 4. 🐞 Fix bug: The `ExtensibleBaseModel`'s `patternProperties` got put both at the top level of the GeoJSON Feature schema and in the "properties" object schema. This would have allowed the input JSON to contain `ext_*` at two levels, which doesn't meet the original schema intent. As part of this bug fix, `ExtensibleBaseModel` is collapsed directly into `OvertureFeature` and the `ext_*` feature is documented as being on a deprecation path. 5. The `validation` package is officially deleted as its essential George functions have all been re-homed either into `system` or `core`. 6. All model constraint mixins have been refactored along the lines started in PR #400. They are now called `@forbid_if`, `@min_fields_set`, `@no_extra_fields`, `@radio_group`, `@require_any_of`, and `@require_if`. 7. A type hint, `Omitable[T]` has been added to the system package. This type hint allows a Pydantic model to "naturally" get JSON Schema semantics for omitted values - it de-conflates optionality and nullability, which Pydantic currently conflates. In an upcoming episode, I hope to replace the `X | None` unions in the existing model with `Omitable[X]`. 8. The existing `Feature` class from the `core` package has been split into two: (i) `Feature` in the `system` package now acts as a generic feature and owns all the funky GeoJSON logic, allowing George enthusiasts to work with a nice ergonomic feature type without having to use an "Overture"-flavored feature; (ii) `OvertureFeature` in the `core` package adds the Overture schema specific flavoring on top of `Feature`. 9. The `ref` module from `core` has been pushed down into `system` and the dependency on `(Overture)Feature` has been factored out in favor of a generic dependency on an `Identified` model, which is basically just a `BaseModel` with a mandatory ID. Stay tuned for episode 4, where we will re-organize the core package.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Nov 26, 2025
The destination is as described in PR #397. In this commit, the following initial steps are taken: 1. Introduce the `system` package and begin migrating the lowest- level building blocks into it from `validation` and `core`. 2. Begin standardizing the Python module approach in a way that will result in fairly ergonomic API reference docs generated by `pdoc`. 3. Fix all broken doctests and include doctests in `make check`. 4. Add a make target for `pydocstyle` with the goal to eventually add it into `make check`. 5. Some renaming and re-organization of types. Stay tuned for episode 2, coming soon to a GitHub near you.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Nov 26, 2025
Re-org. Pydantic packages (2/N) The destination is as described in PR #397 (1/N) and PR #400. In this commit, the following value is added: 1. 🐞 Fix bug: The `@any_of` decorator was not working with transporation destinations. The bug was discovered while refactoring the `mixin` module and the fix is reflected in the updated transportation schema baseline. 2. 🐞 Fix bug: The `make doctest` target exited with code 0 (success) even if it found doctest failures. This bug was added in #397 when the `doctest` target was added and is now fixed. 3. Apply multiple suggestions from PR #397 including renaming `CountryCode` to `CountryCodeAlpha2` and dropping an unnecessary attribute from `PatternConstraint`. 4. Rename the sources package to annex in alignment as shown in the destination package diagram. 5. Begin refactoring `mixin` module to fix some known issues and polish it for final release. Changes made here: - Underlying implementation is replaced with the Pydantic function `create_model` (as was done in the scoping POC) which eliminates the need for `ConstrainedBaseModel` and should also eliminate MRO issues. - Underlying implementation uses some basic "JSON Schema algebra" to ensure that multiple decorators can coexist without breaking each others' JSON Schema changes. - The module name `mixin` is replaced by `model_constraint`, which feels more use case-oriented. - Initially one main decorator is migrated from `mixin` in the validation package to `model_constraint` in the system package. The rest will come next PR. 6. Migrate much of the validation package `README.md` into the system package's main module `__init__.py` and migrate the examples into doctests. 7. Re-home a few other bits of functionality from validation to core. 8. Bring the https://github.com/vcschapp/overture-schema-pydantic scoping into core as `scoping.py` and fix up the linter errors. This is not wired up yet, but that's Coming Soon, likely in episode 4. 9. At Seth's suggestion, switch `ConfidenceScore` from `float64` to `float32` though we will have to double-check the actual value in the Parquet model here. Stay tuned for episode 3, where we will finish re-homing the validation package.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Nov 26, 2025
The destination is as described in PR #397 (1/N), PR #400, and PR #403. In this commit, the following value is added: 1. 🐞 Fix bug: The `not_required_if` mixin wasn't meeting the original schema intent, which result in the `DivisionBoundary` feature type having the wrong JSON Schema. The expected behavior is "if the boundary is at the country level, then the `country` pair should be omitted" but instead it did two variations: in the Python validator, it required the country pair to be present; and in the JSON Schema, it required the country pair to be present, but only if the condition was not true. Both were subtly wrong. The bug is fixed in this commit and the mixin is renamed to `forbid_if`, which I hope will make the semantics more obvious. 2. 🐞 Fix bug: Very subtle, but the mixin model constraint validators did not work correctly if added directly to a `Feature` model, because they aren't aware of the quirky multi-tiered GeoJSON JSON Schema where most properties get pushed down into "properties", but some do not. This is (hopefully ... 🤞) fixed in the new `Feature` model in the system package. 3. 🐞 Fix bug: Very subtle, but the previous `Feature` model's JSON Schema did not prohibit fields named `"id"`, `"bbox"`, and `"geometry"` within the properties object. This created a weird quirk where in a poisoned JSON object containing those values under properties, the properties version would overwrite the root version. This is fixed by explicitly prohibiting those fields in the JSON Schema for the `Feature` class' properties object. 4. 🐞 Fix bug: The `ExtensibleBaseModel`'s `patternProperties` got put both at the top level of the GeoJSON Feature schema and in the "properties" object schema. This would have allowed the input JSON to contain `ext_*` at two levels, which doesn't meet the original schema intent. As part of this bug fix, `ExtensibleBaseModel` is collapsed directly into `OvertureFeature` and the `ext_*` feature is documented as being on a deprecation path. 5. The `validation` package is officially deleted as its essential George functions have all been re-homed either into `system` or `core`. 6. All model constraint mixins have been refactored along the lines started in PR #400. They are now called `@forbid_if`, `@min_fields_set`, `@no_extra_fields`, `@radio_group`, `@require_any_of`, and `@require_if`. 7. A type hint, `Omitable[T]` has been added to the system package. This type hint allows a Pydantic model to "naturally" get JSON Schema semantics for omitted values - it de-conflates optionality and nullability, which Pydantic currently conflates. In an upcoming episode, I hope to replace the `X | None` unions in the existing model with `Omitable[X]`. 8. The existing `Feature` class from the `core` package has been split into two: (i) `Feature` in the `system` package now acts as a generic feature and owns all the funky GeoJSON logic, allowing George enthusiasts to work with a nice ergonomic feature type without having to use an "Overture"-flavored feature; (ii) `OvertureFeature` in the `core` package adds the Overture schema specific flavoring on top of `Feature`. 9. The `ref` module from `core` has been pushed down into `system` and the dependency on `(Overture)Feature` has been factored out in favor of a generic dependency on an `Identified` model, which is basically just a `BaseModel` with a mandatory ID. Stay tuned for episode 4, where we will re-organize the core package.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Dec 4, 2025
The destination is as described in PR #397. In this commit, the following initial steps are taken: 1. Introduce the `system` package and begin migrating the lowest- level building blocks into it from `validation` and `core`. 2. Begin standardizing the Python module approach in a way that will result in fairly ergonomic API reference docs generated by `pdoc`. 3. Fix all broken doctests and include doctests in `make check`. 4. Add a make target for `pydocstyle` with the goal to eventually add it into `make check`. 5. Some renaming and re-organization of types. Stay tuned for episode 2, coming soon to a GitHub near you.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Dec 4, 2025
Re-org. Pydantic packages (2/N) The destination is as described in PR #397 (1/N) and PR #400. In this commit, the following value is added: 1. 🐞 Fix bug: The `@any_of` decorator was not working with transporation destinations. The bug was discovered while refactoring the `mixin` module and the fix is reflected in the updated transportation schema baseline. 2. 🐞 Fix bug: The `make doctest` target exited with code 0 (success) even if it found doctest failures. This bug was added in #397 when the `doctest` target was added and is now fixed. 3. Apply multiple suggestions from PR #397 including renaming `CountryCode` to `CountryCodeAlpha2` and dropping an unnecessary attribute from `PatternConstraint`. 4. Rename the sources package to annex in alignment as shown in the destination package diagram. 5. Begin refactoring `mixin` module to fix some known issues and polish it for final release. Changes made here: - Underlying implementation is replaced with the Pydantic function `create_model` (as was done in the scoping POC) which eliminates the need for `ConstrainedBaseModel` and should also eliminate MRO issues. - Underlying implementation uses some basic "JSON Schema algebra" to ensure that multiple decorators can coexist without breaking each others' JSON Schema changes. - The module name `mixin` is replaced by `model_constraint`, which feels more use case-oriented. - Initially one main decorator is migrated from `mixin` in the validation package to `model_constraint` in the system package. The rest will come next PR. 6. Migrate much of the validation package `README.md` into the system package's main module `__init__.py` and migrate the examples into doctests. 7. Re-home a few other bits of functionality from validation to core. 8. Bring the https://github.com/vcschapp/overture-schema-pydantic scoping into core as `scoping.py` and fix up the linter errors. This is not wired up yet, but that's Coming Soon, likely in episode 4. 9. At Seth's suggestion, switch `ConfidenceScore` from `float64` to `float32` though we will have to double-check the actual value in the Parquet model here. Stay tuned for episode 3, where we will finish re-homing the validation package.
Victor Schappert (vcschapp)
added a commit
that referenced
this pull request
Dec 4, 2025
The destination is as described in PR #397 (1/N), PR #400, and PR #403. In this commit, the following value is added: 1. 🐞 Fix bug: The `not_required_if` mixin wasn't meeting the original schema intent, which result in the `DivisionBoundary` feature type having the wrong JSON Schema. The expected behavior is "if the boundary is at the country level, then the `country` pair should be omitted" but instead it did two variations: in the Python validator, it required the country pair to be present; and in the JSON Schema, it required the country pair to be present, but only if the condition was not true. Both were subtly wrong. The bug is fixed in this commit and the mixin is renamed to `forbid_if`, which I hope will make the semantics more obvious. 2. 🐞 Fix bug: Very subtle, but the mixin model constraint validators did not work correctly if added directly to a `Feature` model, because they aren't aware of the quirky multi-tiered GeoJSON JSON Schema where most properties get pushed down into "properties", but some do not. This is (hopefully ... 🤞) fixed in the new `Feature` model in the system package. 3. 🐞 Fix bug: Very subtle, but the previous `Feature` model's JSON Schema did not prohibit fields named `"id"`, `"bbox"`, and `"geometry"` within the properties object. This created a weird quirk where in a poisoned JSON object containing those values under properties, the properties version would overwrite the root version. This is fixed by explicitly prohibiting those fields in the JSON Schema for the `Feature` class' properties object. 4. 🐞 Fix bug: The `ExtensibleBaseModel`'s `patternProperties` got put both at the top level of the GeoJSON Feature schema and in the "properties" object schema. This would have allowed the input JSON to contain `ext_*` at two levels, which doesn't meet the original schema intent. As part of this bug fix, `ExtensibleBaseModel` is collapsed directly into `OvertureFeature` and the `ext_*` feature is documented as being on a deprecation path. 5. The `validation` package is officially deleted as its essential George functions have all been re-homed either into `system` or `core`. 6. All model constraint mixins have been refactored along the lines started in PR #400. They are now called `@forbid_if`, `@min_fields_set`, `@no_extra_fields`, `@radio_group`, `@require_any_of`, and `@require_if`. 7. A type hint, `Omitable[T]` has been added to the system package. This type hint allows a Pydantic model to "naturally" get JSON Schema semantics for omitted values - it de-conflates optionality and nullability, which Pydantic currently conflates. In an upcoming episode, I hope to replace the `X | None` unions in the existing model with `Omitable[X]`. 8. The existing `Feature` class from the `core` package has been split into two: (i) `Feature` in the `system` package now acts as a generic feature and owns all the funky GeoJSON logic, allowing George enthusiasts to work with a nice ergonomic feature type without having to use an "Overture"-flavored feature; (ii) `OvertureFeature` in the `core` package adds the Overture schema specific flavoring on top of `Feature`. 9. The `ref` module from `core` has been pushed down into `system` and the dependency on `(Overture)Feature` has been factored out in favor of a generic dependency on an `Identified` model, which is basically just a `BaseModel` with a mandatory ID. Stay tuned for episode 4, where we will re-organize the core package.
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.
tl;dr
This is part #1 only. Part #2 will follow fast.
Yes, it leaves things in a mess (temporarily). This is mainly to produce a manageable diff and let us get some alignment on the destination.
Let's ship this quickly so the process doesn't bog down. The end goal is a full and final organization of the packages and the end of churn.
Destination
pyproject.tomlwill be standardized.make check.pydocstylewith the NumPy convention. (As of now, the tool reports too many complaints to add it intomake check.)pydocstylewill be enforced bymake check.docformatterwill be run bymake check.__init__.pydocstring.doctarget in theMakefilethat doesuv run pdocto a useful place.BaseModelbase class insystemwill be given an optionaldescriptionproperty that can be used to provide class-level documentation for the schema user persona. (See this conversation in the comments on this PR.)feature, which will contain the feature models and the main simple types that support them.perspectives.Packaging Deep Dive
The target layout is as shown below.
overture-schema-system
The
systempackage is at the base of the pyramid, and will contain the stuff, otherwise known as "George", that is maximally reusable in any data modeling environment, not just Overture.systemwas chosen after some reflection to try to symbolize something "more basic thancore".foundation, has the unfortunate characteristic of being part of the name Overture Maps Foundation, so was dropped.validationpackage will end up insystem, but some will also migrate up intocore.corepackage will migrate down intosystem.overture-schema-core
The
corepackage contains the shared types that are specific to Overture data. It will continue more or less unchanged, but some of its contents will migrate down tosystem.overture-schema-annex
The
annexpackage will contain "all the other Overture schemas" part from the main geospatial data. The content from thesourcespackage will move in here.overture-schema-***-theme
The
themepackages will remain as-is, subject to: