From 77d3ff967a75c69ea9b02ad86b5504a8ec7dcf38 Mon Sep 17 00:00:00 2001 From: schapper Date: Thu, 30 Jul 2026 17:32:31 -0700 Subject: [PATCH 1/3] Remove annex package Signed-off-by: schapper --- .github/workflows/scripts/package-versions.py | 2 +- packages/overture-schema-annex/README.md | 23 -- packages/overture-schema-annex/pyproject.toml | 37 --- .../src/overture/__init__.py | 1 - .../src/overture/schema/__init__.py | 1 - .../src/overture/schema/annex/__init__.py | 7 - .../src/overture/schema/annex/enums.py | 15 - .../src/overture/schema/annex/models.py | 183 ----------- .../src/overture/schema/annex/types.py | 11 - .../src/overture/schema/py.typed | 0 .../tests/sources_baseline_schema.json | 297 ------------------ .../test_sources_json_schema_baseline.py | 14 - pyproject.toml | 1 - .../sources/coverage-bbox-too-short.yaml | 37 --- .../annex/sources/invalid-data-url.yaml | 17 - .../examples/annex/sources/basic-sources.yaml | 70 ----- reference/examples/annex/sources/minimal.yaml | 17 - uv.lock | 20 +- 18 files changed, 2 insertions(+), 751 deletions(-) delete mode 100644 packages/overture-schema-annex/README.md delete mode 100644 packages/overture-schema-annex/pyproject.toml delete mode 100644 packages/overture-schema-annex/src/overture/__init__.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/__init__.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/annex/__init__.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/annex/enums.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/annex/models.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/annex/types.py delete mode 100644 packages/overture-schema-annex/src/overture/schema/py.typed delete mode 100644 packages/overture-schema-annex/tests/sources_baseline_schema.json delete mode 100644 packages/overture-schema-annex/tests/test_sources_json_schema_baseline.py delete mode 100644 reference/counterexamples/annex/sources/coverage-bbox-too-short.yaml delete mode 100644 reference/counterexamples/annex/sources/invalid-data-url.yaml delete mode 100644 reference/examples/annex/sources/basic-sources.yaml delete mode 100644 reference/examples/annex/sources/minimal.yaml diff --git a/.github/workflows/scripts/package-versions.py b/.github/workflows/scripts/package-versions.py index a0080f95d..575827218 100755 --- a/.github/workflows/scripts/package-versions.py +++ b/.github/workflows/scripts/package-versions.py @@ -63,7 +63,7 @@ def level(package: str) -> int: return 0 elif package in ["overture-schema-common", "overture-schema-core"]: return 1 - elif re.fullmatch(r'overture-schema-.*-theme', package) or package in ["overture-schema", "overture-schema-cli", "overture-schema-codegen", "overture-schema-annex", "overture-schema-pyspark"]: + elif re.fullmatch(r'overture-schema-.*-theme', package) or package in ["overture-schema", "overture-schema-cli", "overture-schema-codegen", "overture-schema-pyspark"]: return 2 else: raise ValueError(f"Unknown package for level computation: {package}") diff --git a/packages/overture-schema-annex/README.md b/packages/overture-schema-annex/README.md deleted file mode 100644 index 2000ea7ad..000000000 --- a/packages/overture-schema-annex/README.md +++ /dev/null @@ -1,23 +0,0 @@ -# Overture Schema Sources - -Shared models and validation tools for the data sources catalog used across -Overture Maps themes. - -## Contents - -- `overture.schema.sources` module, exporting the `Sources` aggregate model - and related dataset structures. -- CLI helper (`python -m overture.schema.sources`) to validate sources JSON - files against the schema. -- Reference examples and counterexamples under `schema/reference` illustrating - valid and invalid source structures. - -## Usage - -```bash -uv run python -m overture.schema.sources path/to/sources.json -``` - -This validates the payload and reports schema violations with dataset-level -context. The package also registers an `overture.models` entry point for the -`Sources` model so tools can discover the schema automatically. \ No newline at end of file diff --git a/packages/overture-schema-annex/pyproject.toml b/packages/overture-schema-annex/pyproject.toml deleted file mode 100644 index af7ea058d..000000000 --- a/packages/overture-schema-annex/pyproject.toml +++ /dev/null @@ -1,37 +0,0 @@ -[project] -maintainers = [ - {name = "Overture Maps Schema Working Group"}, -] -dependencies = ["overture-schema-common", "overture-schema-system", "pydantic>=2.12.0"] -description = "Add your description here" -version = "0.1.1" -license = "MIT" -name = "overture-schema-annex" -readme = "README.md" -requires-python = ">=3.10" - -[tool.uv.sources] -overture-schema-common = { workspace = true } -overture-schema-system = { workspace = true } - -[project.urls] -Homepage = "https://overturemaps.org" -Source = "https://github.com/OvertureMaps/schema" -Issues = "https://github.com/OvertureMaps/schema/issues" - -[build-system] -build-backend = "hatchling.build" -requires = ["hatchling"] - -[tool.hatch.build.targets.wheel] -packages = ["src/overture"] - -[project.entry-points."overture.models"] -sources = "overture.schema.annex:Sources" - -[project.entry-points.pytest11] -overture_baselines = "overture.schema.system.testing.plugin" - -[tool.pytest.ini_options] -pythonpath = ["src"] -testpaths = ["tests"] diff --git a/packages/overture-schema-annex/src/overture/__init__.py b/packages/overture-schema-annex/src/overture/__init__.py deleted file mode 100644 index 8db66d3d0..000000000 --- a/packages/overture-schema-annex/src/overture/__init__.py +++ /dev/null @@ -1 +0,0 @@ -__path__ = __import__("pkgutil").extend_path(__path__, __name__) diff --git a/packages/overture-schema-annex/src/overture/schema/__init__.py b/packages/overture-schema-annex/src/overture/schema/__init__.py deleted file mode 100644 index 8db66d3d0..000000000 --- a/packages/overture-schema-annex/src/overture/schema/__init__.py +++ /dev/null @@ -1 +0,0 @@ -__path__ = __import__("pkgutil").extend_path(__path__, __name__) diff --git a/packages/overture-schema-annex/src/overture/schema/annex/__init__.py b/packages/overture-schema-annex/src/overture/schema/annex/__init__.py deleted file mode 100644 index 8dddb62b3..000000000 --- a/packages/overture-schema-annex/src/overture/schema/annex/__init__.py +++ /dev/null @@ -1,7 +0,0 @@ -"""Sources schema package.""" - -__path__ = __import__("pkgutil").extend_path(__path__, __name__) - -from .models import Sources - -__all__ = ["Sources"] diff --git a/packages/overture-schema-annex/src/overture/schema/annex/enums.py b/packages/overture-schema-annex/src/overture/schema/annex/enums.py deleted file mode 100644 index 0ed0188ba..000000000 --- a/packages/overture-schema-annex/src/overture/schema/annex/enums.py +++ /dev/null @@ -1,15 +0,0 @@ -from enum import Enum - - -class BuildSource(str, Enum): - """The ingest source for address data.""" - - OPEN_ADDRESSES = "OpenAddresses" - TF_DATA_PLATFORM = "tf-data-platform" - - -class UpdateType(str, Enum): - """Whether the data is continuously updated upstream or needs manual intervention.""" - - CONTINUOUS = "continuous" - MANUAL = "manual" diff --git a/packages/overture-schema-annex/src/overture/schema/annex/models.py b/packages/overture-schema-annex/src/overture/schema/annex/models.py deleted file mode 100644 index 1e3cb041d..000000000 --- a/packages/overture-schema-annex/src/overture/schema/annex/models.py +++ /dev/null @@ -1,183 +0,0 @@ -"""Sources schema models for Overture Maps data sources.""" - -from datetime import date -from typing import Annotated, Literal - -from pydantic import BaseModel, Field, HttpUrl - -from overture.schema.system.model_constraint import no_extra_fields -from overture.schema.system.string import CountryCodeAlpha2 - -from .enums import BuildSource, UpdateType -from .types import LicenseShortname - - -@no_extra_fields -class Dataset(BaseModel): - """Dataset definition for Overture Maps data sources.""" - - # Required - - source_name: Annotated[ - str, - Field(description="The name of the source."), - ] - source_dataset_name: Annotated[ - str, - Field( - description="The name of the dataset being used from the source. This should match the 'dataset' value found in a record's sources column." - ), - ] - data_url: Annotated[ - HttpUrl | Literal[""], - Field( - description="The data page or data portal of this source, typically includes links to data downloads and license links.", - ), - ] - data_url_archived: Annotated[ - HttpUrl | Literal[""], - Field( - description="URL of the source's data page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - ), - ] - license_url: Annotated[ - HttpUrl | Literal[""], - Field( - description="A link to this source's data license or page referencing the license associated with the data being imported. This should include explicit license terms.", - ), - ] - license_url_archived: Annotated[ - HttpUrl | Literal[""], - Field( - description="URL of the source's license page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - ), - ] - license_type: Annotated[ - str, - Field( - description="The license that is associated with the data being used from this source. This should be a valid SPDX license identifier when available." - ), - ] - license_text: Annotated[ - str, - Field( - description="Any relevant license text, direct from the source's license page." - ), - ] - license_attribution: Annotated[ - str, - Field(description="Any attribution required by this source."), - ] - # FIXME: This should be a `BBox` primitive, not a `list[float]`. - coverage_bbox: Annotated[ - list[float], - Field( - description="The bounding box, in [xmin, ymin, xmax, ymax] format, of this source's coverage.", - min_length=4, - max_length=4, - ), - ] - - # Optional - - inception_date: Annotated[ - date | None, - Field( - description="The first date this source was used in the Overture addresses theme, in YYYY-MM-DD format." - ), - ] = None - url: Annotated[ - HttpUrl | None, - Field(description="The home page of this source."), - ] = None - url_archived: Annotated[ - HttpUrl | None, - Field( - description="URL of the source's home page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - ), - ] = None - data_download_url: Annotated[ - list[HttpUrl | Literal[""]] | None, - Field( - description="Either a direct download link of data from this source, typically a geo-format or compressed file, or an endpoint from where the data was obtained for use within Overture." - ), - ] = None - countries: Annotated[ - list[CountryCodeAlpha2 | Literal["Global"]] | None, - Field( - description="A list of two-character iso country codes that this data source provides data in." - ), - ] = None - coverage_description: Annotated[ - str | None, - Field( - description="A description of the coverage type of the source data - i.e. national, regional, local." - ), - ] = None - data_layer_name: Annotated[ - str | None, - Field(description="Name of the data layer used from this source."), - ] = None - oa_path: Annotated[ - list[str] | None, - Field(description="File path and name in OpenAddresses, if existing."), - ] = None - address_levels: Annotated[ - list[str] | None, - Field( - description="Available address level attributes from OpenAddress, if existing." - ), - ] = None - file_format: Annotated[ - str | None, - Field(description="Format of the file used from this source."), - ] = None - update_frequency: Annotated[ - str | None, - Field(description="How frequently the source data is updated upstream."), - ] = None - build_source: BuildSource | None = None - update_type: UpdateType | None = None - update_schedule: Annotated[ - list[str] | None, - Field( - description="The month or months in which the data is to be re-ingested by the Overture theme using this data source." - ), - ] = None - known_issues: Annotated[ - str | None, - Field( - description="A description of any issues with the data that are known - i.e. data is incomplete, coverage is incomplete, or issues with character encoding." - ), - ] = None - notes: Annotated[ - str | None, - Field( - description="Freeform notes about this data source, including notes on any pre-processing requirements." - ), - ] = None - requires_attribution: Annotated[ - str | None, # TODO should this be a bool? - Field( - description="Whether this source requires attribution to be used in Overture Maps." - ), - ] = None - - -@no_extra_fields -class Sources(BaseModel): - """Common schema definitions for data sources.""" - - # Required - - datasets: Annotated[ - list[Dataset], - Field(description="List of data source entries used by Overture."), - ] - license_priority: Annotated[ - dict[LicenseShortname, Annotated[int, Field(ge=0)]], - Field( - description="Map of license shortnames to their priority (lower number indicates higher priority)." - ), - Field(json_schema_extra={"additionalProperties": False}), - ] diff --git a/packages/overture-schema-annex/src/overture/schema/annex/types.py b/packages/overture-schema-annex/src/overture/schema/annex/types.py deleted file mode 100644 index 64dc3626a..000000000 --- a/packages/overture-schema-annex/src/overture/schema/annex/types.py +++ /dev/null @@ -1,11 +0,0 @@ -from typing import Annotated, NewType - -from pydantic import Field - -LicenseShortname = NewType( - "LicenseShortname", - Annotated[ - str, - Field(pattern=r"^[A-Za-z0-9._+\-]+$"), - ], -) diff --git a/packages/overture-schema-annex/src/overture/schema/py.typed b/packages/overture-schema-annex/src/overture/schema/py.typed deleted file mode 100644 index e69de29bb..000000000 diff --git a/packages/overture-schema-annex/tests/sources_baseline_schema.json b/packages/overture-schema-annex/tests/sources_baseline_schema.json deleted file mode 100644 index f39b00922..000000000 --- a/packages/overture-schema-annex/tests/sources_baseline_schema.json +++ /dev/null @@ -1,297 +0,0 @@ -{ - "$defs": { - "BuildSource": { - "description": "The ingest source for address data.", - "enum": [ - "OpenAddresses", - "tf-data-platform" - ], - "title": "BuildSource", - "type": "string" - }, - "Dataset": { - "additionalProperties": false, - "description": "Dataset definition for Overture Maps data sources.", - "properties": { - "address_levels": { - "description": "Available address level attributes from OpenAddress, if existing.", - "items": { - "type": "string" - }, - "title": "Address Levels", - "type": "array" - }, - "build_source": { - "$ref": "#/$defs/BuildSource" - }, - "countries": { - "description": "A list of two-character iso country codes that this data source provides data in.", - "items": { - "anyOf": [ - { - "description": "ISO 3166-1 alpha-2 country code", - "maxLength": 2, - "minLength": 2, - "pattern": "^[A-Z]{2}$", - "type": "string" - }, - { - "const": "Global", - "type": "string" - } - ] - }, - "title": "Countries", - "type": "array" - }, - "coverage_bbox": { - "description": "The bounding box, in [xmin, ymin, xmax, ymax] format, of this source's coverage.", - "items": { - "type": "number" - }, - "maxItems": 4, - "minItems": 4, - "title": "Coverage Bbox", - "type": "array" - }, - "coverage_description": { - "description": "A description of the coverage type of the source data - i.e. national, regional, local.", - "title": "Coverage Description", - "type": "string" - }, - "data_download_url": { - "description": "Either a direct download link of data from this source, typically a geo-format or compressed file, or an endpoint from where the data was obtained for use within Overture.", - "items": { - "anyOf": [ - { - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "type": "string" - }, - { - "const": "", - "type": "string" - } - ] - }, - "title": "Data Download Url", - "type": "array" - }, - "data_layer_name": { - "description": "Name of the data layer used from this source.", - "title": "Data Layer Name", - "type": "string" - }, - "data_url": { - "anyOf": [ - { - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "type": "string" - }, - { - "const": "", - "type": "string" - } - ], - "description": "The data page or data portal of this source, typically includes links to data downloads and license links.", - "title": "Data Url" - }, - "data_url_archived": { - "anyOf": [ - { - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "type": "string" - }, - { - "const": "", - "type": "string" - } - ], - "description": "URL of the source's data page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - "title": "Data Url Archived" - }, - "file_format": { - "description": "Format of the file used from this source.", - "title": "File Format", - "type": "string" - }, - "inception_date": { - "description": "The first date this source was used in the Overture addresses theme, in YYYY-MM-DD format.", - "format": "date", - "title": "Inception Date", - "type": "string" - }, - "known_issues": { - "description": "A description of any issues with the data that are known - i.e. data is incomplete, coverage is incomplete, or issues with character encoding.", - "title": "Known Issues", - "type": "string" - }, - "license_attribution": { - "description": "Any attribution required by this source.", - "title": "License Attribution", - "type": "string" - }, - "license_text": { - "description": "Any relevant license text, direct from the source's license page.", - "title": "License Text", - "type": "string" - }, - "license_type": { - "description": "The license that is associated with the data being used from this source. This should be a valid SPDX license identifier when available.", - "title": "License Type", - "type": "string" - }, - "license_url": { - "anyOf": [ - { - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "type": "string" - }, - { - "const": "", - "type": "string" - } - ], - "description": "A link to this source's data license or page referencing the license associated with the data being imported. This should include explicit license terms.", - "title": "License Url" - }, - "license_url_archived": { - "anyOf": [ - { - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "type": "string" - }, - { - "const": "", - "type": "string" - } - ], - "description": "URL of the source's license page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - "title": "License Url Archived" - }, - "notes": { - "description": "Freeform notes about this data source, including notes on any pre-processing requirements.", - "title": "Notes", - "type": "string" - }, - "oa_path": { - "description": "File path and name in OpenAddresses, if existing.", - "items": { - "type": "string" - }, - "title": "Oa Path", - "type": "array" - }, - "requires_attribution": { - "description": "Whether this source requires attribution to be used in Overture Maps.", - "title": "Requires Attribution", - "type": "string" - }, - "source_dataset_name": { - "description": "The name of the dataset being used from the source. This should match the 'dataset' value found in a record's sources column.", - "title": "Source Dataset Name", - "type": "string" - }, - "source_name": { - "description": "The name of the source.", - "title": "Source Name", - "type": "string" - }, - "update_frequency": { - "description": "How frequently the source data is updated upstream.", - "title": "Update Frequency", - "type": "string" - }, - "update_schedule": { - "description": "The month or months in which the data is to be re-ingested by the Overture theme using this data source.", - "items": { - "type": "string" - }, - "title": "Update Schedule", - "type": "array" - }, - "update_type": { - "$ref": "#/$defs/UpdateType" - }, - "url": { - "description": "The home page of this source.", - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "title": "Url", - "type": "string" - }, - "url_archived": { - "description": "URL of the source's home page, stored on archive.org, at or near the date the source data was obtained for use within Overture.", - "format": "uri", - "maxLength": 2083, - "minLength": 1, - "title": "Url Archived", - "type": "string" - } - }, - "required": [ - "source_name", - "source_dataset_name", - "data_url", - "data_url_archived", - "license_url", - "license_url_archived", - "license_type", - "license_text", - "license_attribution", - "coverage_bbox" - ], - "title": "Dataset", - "type": "object" - }, - "UpdateType": { - "description": "Whether the data is continuously updated upstream or needs manual intervention.", - "enum": [ - "continuous", - "manual" - ], - "title": "UpdateType", - "type": "string" - } - }, - "additionalProperties": false, - "description": "Common schema definitions for data sources.", - "properties": { - "datasets": { - "description": "List of data source entries used by Overture.", - "items": { - "$ref": "#/$defs/Dataset" - }, - "title": "Datasets", - "type": "array" - }, - "license_priority": { - "additionalProperties": false, - "description": "Map of license shortnames to their priority (lower number indicates higher priority).", - "patternProperties": { - "^[A-Za-z0-9._+\\-]+$": { - "minimum": 0, - "type": "integer" - } - }, - "title": "License Priority", - "type": "object" - } - }, - "required": [ - "datasets", - "license_priority" - ], - "title": "Sources", - "type": "object" -} \ No newline at end of file diff --git a/packages/overture-schema-annex/tests/test_sources_json_schema_baseline.py b/packages/overture-schema-annex/tests/test_sources_json_schema_baseline.py deleted file mode 100644 index 3ab9e9e48..000000000 --- a/packages/overture-schema-annex/tests/test_sources_json_schema_baseline.py +++ /dev/null @@ -1,14 +0,0 @@ -"""Golden JSON Schema test for Sources type.""" - -from pathlib import Path - -import pytest -from overture.schema.annex import Sources -from overture.schema.system.testing import assert_json_schema_golden - -GOLDEN = Path(__file__).parent / "sources_baseline_schema.json" - - -@pytest.mark.baseline -def test_sources_json_schema(update_baselines: bool) -> None: - assert_json_schema_golden(Sources, GOLDEN, update=update_baselines) diff --git a/pyproject.toml b/pyproject.toml index ea86feca4..3439b87f6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,7 +65,6 @@ markers = [ ] pythonpath = [ "packages/overture-schema-addresses-theme/tests", - "packages/overture-schema-annex/tests", "packages/overture-schema-base-theme/tests", "packages/overture-schema-buildings-theme/tests", "packages/overture-schema-cli/tests", diff --git a/reference/counterexamples/annex/sources/coverage-bbox-too-short.yaml b/reference/counterexamples/annex/sources/coverage-bbox-too-short.yaml deleted file mode 100644 index e2b9c79db..000000000 --- a/reference/counterexamples/annex/sources/coverage-bbox-too-short.yaml +++ /dev/null @@ -1,37 +0,0 @@ -datasets: - - source_name: Example City Open Data - source_dataset_name: address_points - data_url: https://data.example.gov/datasets/address-points - data_url_archived: https://web.archive.org/web/2025/https://data.example.gov/datasets/address-points - license_url: https://data.example.gov/license - license_url_archived: https://web.archive.org/web/2025/https://data.example.gov/license - license_type: CC-BY-4.0 - license_text: Example text - license_attribution: City of Example GIS Department - coverage_bbox: - - -122.79 - - 45.43 - - -122.45 - data_download_url: - - https://data.example.gov/downloads/address_points.geojson - countries: - - US - coverage_description: Citywide coverage - update_frequency: monthly - - source_name: Example Parcel Authority - source_dataset_name: parcel_boundaries - data_url: https://parcels.example.gov/downloads/boundaries - data_url_archived: https://web.archive.org/web/2025/https://parcels.example.gov/downloads/boundaries - license_url: https://parcels.example.gov/license - license_url_archived: https://web.archive.org/web/2025/https://parcels.example.gov/license - license_type: CC0-1.0 - license_text: Parcel license text - license_attribution: Example Parcel Authority - coverage_bbox: - - -122.81 - - 45.38 - - -122.37 - - 45.72 -license_priority: - CC-BY-4.0: 1 - CC0-1.0: 2 diff --git a/reference/counterexamples/annex/sources/invalid-data-url.yaml b/reference/counterexamples/annex/sources/invalid-data-url.yaml deleted file mode 100644 index d5d300e59..000000000 --- a/reference/counterexamples/annex/sources/invalid-data-url.yaml +++ /dev/null @@ -1,17 +0,0 @@ -datasets: - - source_name: Example Broken URLs - source_dataset_name: address_points - data_url: not-a-valid-url - data_url_archived: https://web.archive.org/web/2024/not-a-valid-url - license_url: not-a-valid-url/license - license_url_archived: https://web.archive.org/web/2024/not-a-valid-url/license - license_type: CC-BY-4.0 - license_text: Broken feed license text - license_attribution: Example Broken URLs - coverage_bbox: - - -123.0 - - 45.0 - - -122.5 - - 45.4 -license_priority: - CC-BY-4.0: 1 diff --git a/reference/examples/annex/sources/basic-sources.yaml b/reference/examples/annex/sources/basic-sources.yaml deleted file mode 100644 index 2c6d10290..000000000 --- a/reference/examples/annex/sources/basic-sources.yaml +++ /dev/null @@ -1,70 +0,0 @@ -datasets: - - source_name: Example City Open Data - source_dataset_name: address_points - data_url: https://data.example.gov/datasets/address-points - data_url_archived: https://web.archive.org/web/2025/https://data.example.gov/datasets/address-points - license_url: https://data.example.gov/license - license_url_archived: https://web.archive.org/web/2025/https://data.example.gov/license - license_type: CC-BY-4.0 - license_text: | - The City of Example grants permission to use this dataset under the terms of the Creative Commons Attribution 4.0 International License. - license_attribution: City of Example GIS Department - coverage_bbox: - - -122.79 - - 45.43 - - -122.45 - - 45.65 - inception_date: '2022-08-01' - url: https://example.gov/ - url_archived: https://web.archive.org/web/2025/https://example.gov - data_download_url: - - https://data.example.gov/downloads/address_points.geojson - countries: - - US - coverage_description: Citywide coverage - data_layer_name: address_points - oa_path: - - us/example_city/addresses.csv - address_levels: - - address - file_format: CSV - update_frequency: monthly - build_source: OpenAddresses - update_type: continuous - update_schedule: - - January - - July - known_issues: Source omits new subdivisions until quarterly update. - notes: Dataset normalized to Overture schema during ingest. - requires_attribution: Yes - - source_name: Example Parcel Authority - source_dataset_name: parcel_boundaries - data_url: https://parcels.example.gov/downloads/boundaries - data_url_archived: https://web.archive.org/web/2025/https://parcels.example.gov/downloads/boundaries - license_url: https://parcels.example.gov/license - license_url_archived: https://web.archive.org/web/2025/https://parcels.example.gov/license - license_type: ODbL-1.0 - license_text: | - This dataset is available under the Open Database License v1.0. - license_attribution: Example Parcel Authority - coverage_bbox: - - -122.81 - - 45.38 - - -122.37 - - 45.72 - url: https://parcels.example.gov/ - data_download_url: - - https://parcels.example.gov/downloads/parcels.gpkg - countries: - - US - coverage_description: Countywide parcel coverage - file_format: GPKG - update_frequency: quarterly - build_source: tf-data-platform - update_type: manual - known_issues: Parcel ownership attributes excluded from public release. - notes: Simplified geometry provided for efficient rendering. - requires_attribution: Required in downstream products. -license_priority: - CC-BY-4.0: 1 - ODbL-1.0: 2 diff --git a/reference/examples/annex/sources/minimal.yaml b/reference/examples/annex/sources/minimal.yaml deleted file mode 100644 index a48862f7e..000000000 --- a/reference/examples/annex/sources/minimal.yaml +++ /dev/null @@ -1,17 +0,0 @@ -datasets: - - source_name: Address Sample - source_dataset_name: addresses_sample - data_url: https://example.com/addresses - data_url_archived: https://web.archive.org/web/2024/https://example.com/addresses - license_url: https://example.com/addresses/license - license_url_archived: https://web.archive.org/web/2024/https://example.com/addresses/license - license_type: CC-BY-4.0 - license_text: This sample dataset is released under CC-BY-4.0. - license_attribution: Addresses Sample Contributors - coverage_bbox: - - -10.0 - - 50.0 - - -9.5 - - 50.5 -license_priority: - CC-BY-4.0: 1 diff --git a/uv.lock b/uv.lock index 65b5c0639..f1eb70fa3 100644 --- a/uv.lock +++ b/uv.lock @@ -16,7 +16,6 @@ exclude-newer-span = "P1W" members = [ "overture-schema", "overture-schema-addresses-theme", - "overture-schema-annex", "overture-schema-base-theme", "overture-schema-buildings-theme", "overture-schema-cli", @@ -363,7 +362,7 @@ name = "exceptiongroup" version = "1.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions" }, + { name = "typing-extensions", marker = "python_full_version < '3.11'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/50/79/66800aadf48771f6b62f7eb014e352e5d06856655206165d775e675a02c9/exceptiongroup-1.3.1.tar.gz", hash = "sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219", size = 30371, upload-time = "2025-11-21T23:01:54.787Z" } wheels = [ @@ -953,23 +952,6 @@ requires-dist = [ { name = "pydantic", specifier = ">=2.12.0" }, ] -[[package]] -name = "overture-schema-annex" -version = "0.1.1" -source = { editable = "packages/overture-schema-annex" } -dependencies = [ - { name = "overture-schema-common" }, - { name = "overture-schema-system" }, - { name = "pydantic" }, -] - -[package.metadata] -requires-dist = [ - { name = "overture-schema-common", editable = "packages/overture-schema-common" }, - { name = "overture-schema-system", editable = "packages/overture-schema-system" }, - { name = "pydantic", specifier = ">=2.12.0" }, -] - [[package]] name = "overture-schema-base-theme" version = "0.1.1" From 935a09aba35989246520ecccebb35ce0d8bd816d Mon Sep 17 00:00:00 2001 From: schapper Date: Thu, 30 Jul 2026 18:02:11 -0700 Subject: [PATCH 2/3] =?UTF-8?q?Fix=20bugs:=20CLI=20validation=20failures?= =?UTF-8?q?=20revealed=20by=20annex=20delete=20=F0=9F=AA=B2=F0=9F=90=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the annex package's `Sources` model left the discovered model union without its last plain (non-discriminated) member, changing pydantic's validation output and exposing two latent bugs in the `overture-schema-cli` error formatter: 1. False "Ambiguous" warning on a clean match: when validating a list against a union that mixes a tagged-union with another discriminated member†, an item that cleanly matched one branch still emitted a `union_tag_not_found` error from the sibling branch. That empty discriminator path error formed a group that tied with the real match, so the CLI wrongly reported "Ambiguous: data matches multiple types equally." Fixed by discarding sibling-branch `union_tag_not_found` noise for any item that already has a concrete error, before tie detection. 2. Opaque message for undiscriminatable items: an item matching no discriminator (e.g. missing `type`) produced only "Unable to extract tag using discriminator" with no field-level detail‡. Fixed by re-validating such an item against each candidate model, selecting the best fit (fewest errors), and surfacing that model's field-level errors under the item's index. ----------- † Simplified example of "validating a list against a union that mixes a tagged union with another discriminated member: ```python from typing import Literal from pydantic import BaseModel, TypeAdapter class Building(BaseModel): type: Literal["building"] id: str class Place(BaseModel): type: Literal["place"] id: str Feature = Building | Place # the UNION adapter = TypeAdapter(list[Feature]) # a LIST of that union adapter.validate_python([ {"type": "building", "id": "b1"}, # element 0 → must be one of {Building, Place} → Building {"type": "place", "id": "p1"}, # element 1 → must be one of {Building, Place} → Place ]) ``` The CLI basically does this against an input JSON array of features. ----------- ‡ Here's a trivial example of the second situation: ```python from typing import Annotated, Literal, Union from pydantic import BaseModel, Field, TypeAdapter, ValidationError class Building(BaseModel): type: Literal["building"] class Place(BaseModel): type: Literal["place"] Feature = Annotated[Union[Building, Place], Field(discriminator="type")] adapter = TypeAdapter(list[Feature]) adapter.validate_python([{}, {}]) ``` Signed-off-by: schapper --- .../src/overture/schema/cli/commands.py | 132 +++++++++++++++- .../overture/schema/cli/error_formatting.py | 89 +++++++++++ .../tests/test_cli_functions.py | 144 +++++++++++++++++- .../tests/test_error_formatting.py | 130 +++++++++++++++- 4 files changed, 489 insertions(+), 6 deletions(-) diff --git a/packages/overture-schema-cli/src/overture/schema/cli/commands.py b/packages/overture-schema-cli/src/overture/schema/cli/commands.py index 24062452e..24ce225f1 100644 --- a/packages/overture-schema-cli/src/overture/schema/cli/commands.py +++ b/packages/overture-schema-cli/src/overture/schema/cli/commands.py @@ -37,7 +37,7 @@ ) from .tag_options import build_selector, tag_selection_options from .type_analysis import StructuralTuple, get_item_index, introspect_union -from .types import ErrorLocation, UnionType +from .types import ErrorLocation, UnionType, ValidationErrorDict # Console instances for rich output stdout = Console(highlight=False) @@ -443,6 +443,119 @@ def print_collection_statistics( stderr.print() +def _best_fit_model( + item_data: object, + candidate_models: tuple[type[BaseModel], ...], +) -> tuple[type[BaseModel], list[ValidationErrorDict]] | None: + """Find the candidate model that a single item best fits. + + Validates `item_data` against each candidate and returns the model with + the fewest validation errors (the fewest changes needed to make the data + valid), together with those errors. Ties are broken deterministically by + candidate order. Returns `None` if no candidate produced errors (nothing + to re-home). + + Parameters + ---------- + item_data : object + The single feature's data (GeoJSON or flat dict). + candidate_models : tuple[type[BaseModel], ...] + Candidate model classes to try, in a stable order. + + Returns + ------- + tuple[type[BaseModel], list[ValidationErrorDict]] | None + A (model, errors) tuple for the best fit, or None if no candidate + produced errors. + """ + best: tuple[int, type[BaseModel], list[ValidationErrorDict]] | None = None + for model in candidate_models: + try: + validate_feature(cast(dict, item_data), model) + except ValidationError as exc: + errs = exc.errors() + else: + # Item validated cleanly against this candidate; it is not the + # source of a failure, so it is not a useful "best fit" to report. + continue + if best is None or len(errs) < best[0]: + best = (len(errs), model, errs) + if best is None: + return None + return best[1], best[2] + + +def _revalidate_undiscriminatable_items( + errors: list[ValidationErrorDict], + original_data: dict | list | None, + candidate_models: tuple[type[BaseModel], ...], +) -> tuple[list[ValidationErrorDict], dict[int, type[BaseModel]]]: + """Replace undiscriminatable-item noise with best-fit field errors. + + When a list item cannot be discriminated (its errors are *entirely* + `union_tag_not_found`), pydantic cannot select a union branch and reports + only the opaque "unable to extract tag" message with no field-level detail, + resulting in useless validation output. + + This function re-validates those non-discriminatable items against each + candidate model, picks the best fitting candidate model (fewest errors, see + `_best_fit_model`), and replaces the item's errors with that model's field + errors. + + Items that already have at least one concrete (non-`union_tag_not_found`) + error are left untouched because for them, pydantic has already produced + useful field-level detail (as happens when the union contains a plain, + non-discriminated member). + + Parameters + ---------- + errors : list[ValidationErrorDict] + The raw validation errors from `ValidationError.errors()`. + original_data : dict | list | None + The original parsed input (only lists are handled here). + candidate_models : tuple[type[BaseModel], ...] + Candidate model classes to try, in a stable order. + + Returns + ------- + tuple[list[ValidationErrorDict], dict[int, type[BaseModel]]] + A tuple of (possibly-augmented errors, {item_index: best_fit_model}). + The mapping is used to label each re-homed item's type in the display. + """ + if not isinstance(original_data, list): + return errors, {} + + errors_by_item: dict[int | None, list[ValidationErrorDict]] = defaultdict(list) + for error in errors: + errors_by_item[get_item_index(error["loc"])].append(error) + + augmented: list[ValidationErrorDict] = [] + best_fit_types: dict[int, type[BaseModel]] = {} + + for item_idx, item_errors in errors_by_item.items(): + if ( + item_idx is None + or not (0 <= item_idx < len(original_data)) + or not all(e.get("type") == "union_tag_not_found" for e in item_errors) + ): + augmented.extend(item_errors) + continue + + best = _best_fit_model(original_data[item_idx], candidate_models) + if best is None: + augmented.extend(item_errors) + continue + + best_model, best_errors = best + best_fit_types[item_idx] = best_model + for err in best_errors: + rehomed = dict(err) + rehomed["loc"] = (item_idx, *err["loc"]) + augmented.append(cast(ValidationErrorDict, rehomed)) + + return augmented, best_fit_types + + def handle_validation_error( e: ValidationError, model_type: UnionType, @@ -474,15 +587,28 @@ def handle_validation_error( # Create cache for structural tuple computation (optimizes systematic errors) structural_cache: dict[ErrorLocation, StructuralTuple] = {} + # For list items that cannot be discriminated at all (errors are entirely + # union_tag_not_found), replace the opaque "unable to extract tag" noise + # with field-level errors from the best-fit candidate model, re-homed under + # the item index so they render as normal field errors. + errors, best_fit_item_types = _revalidate_undiscriminatable_items( + e.errors(), + original_data, + tuple(dict.fromkeys(metadata.discriminator_to_model.values())), + ) + # Group errors by discriminator path and select most likely group(s) - error_groups = group_errors_by_discriminator(e.errors(), metadata, structural_cache) + error_groups = group_errors_by_discriminator(errors, metadata, structural_cache) filtered_errors, is_tied, is_heterogeneous, item_types = select_most_likely_errors( error_groups, metadata=metadata, - all_errors=e.errors(), + all_errors=errors, structural_cache=structural_cache, ) + # Label re-homed best-fit items with their inferred type. + item_types.update(best_fit_item_types) + # Show heterogeneity warning if collection has mixed types if is_heterogeneous: stderr.print( diff --git a/packages/overture-schema-cli/src/overture/schema/cli/error_formatting.py b/packages/overture-schema-cli/src/overture/schema/cli/error_formatting.py index b8de95d67..0667c5f7d 100644 --- a/packages/overture-schema-cli/src/overture/schema/cli/error_formatting.py +++ b/packages/overture-schema-cli/src/overture/schema/cli/error_formatting.py @@ -157,6 +157,84 @@ def analyze_collection_heterogeneity( return item_types, is_heterogeneous +def _suppress_sibling_tag_noise( + error_groups: dict[ErrorLocation, list[ValidationErrorDict]], +) -> dict[ErrorLocation, list[ValidationErrorDict]]: + """Drop `union_tag_not_found` noise for items that already matched a type. + + When validating against a union that mixes a tagged-union with other + (also discriminated) members, an item that cleanly matches one branch still + produces a `union_tag_not_found` error from every *sibling* branch whose + discriminator it fails to satisfy. Those errors carry an empty discriminator + path `()`, forming a spurious group that can tie with the real match and + trigger a false "Ambiguous" warning. + + This helper removes such noise for any list item that already has a concrete + (non-`union_tag_not_found`) error. Items whose *only* errors are + `union_tag_not_found` are left untouched, so genuinely undiscriminatable + input is still reported. Groups left empty after filtering are dropped. + + Parameters + ---------- + error_groups : dict[ErrorLocation, list[ValidationErrorDict]] + Mapping of discriminator paths to their error lists. + + Returns + ------- + dict[ErrorLocation, list[ValidationErrorDict]] + A new error-groups dict with sibling-branch tag noise removed. + + Examples + -------- + >>> # Item 0 matched 'building' but the sibling 'segment' branch emitted + >>> # a union_tag_not_found error, creating a spurious () group. + >>> error_groups = { + ... ('building',): [ + ... {'loc': (0, 'tagged-union[type]', 'building', 'id'), + ... 'msg': 'Field required', 'type': 'missing'}, + ... ], + ... (): [ + ... {'loc': (0, 'tagged-union[subtype]'), + ... 'msg': 'Unable to extract tag', 'type': 'union_tag_not_found'}, + ... ], + ... } + >>> cleaned = _suppress_sibling_tag_noise(error_groups) + >>> list(cleaned.keys()) # the () noise group is gone + [('building',)] + + >>> # An item whose ONLY error is union_tag_not_found is preserved. + >>> error_groups = { + ... (): [ + ... {'loc': (2, 'tagged-union[type]'), + ... 'msg': 'Unable to extract tag', 'type': 'union_tag_not_found'}, + ... ], + ... } + >>> _suppress_sibling_tag_noise(error_groups) == error_groups + True + """ + # Identify which items have at least one concrete (non tag-not-found) error. + items_with_concrete_error: set[int | None] = set() + for errors in error_groups.values(): + for error in errors: + if error.get("type") != "union_tag_not_found": + items_with_concrete_error.add(get_item_index(error["loc"])) + + cleaned: dict[ErrorLocation, list[ValidationErrorDict]] = {} + for disc_path, errors in error_groups.items(): + kept = [ + error + for error in errors + if not ( + error.get("type") == "union_tag_not_found" + and get_item_index(error["loc"]) in items_with_concrete_error + ) + ] + if kept: + cleaned[disc_path] = kept + + return cleaned + + def select_most_likely_errors( error_groups: dict[ErrorLocation, list[ValidationErrorDict]], metadata: UnionMetadata | None = None, @@ -171,6 +249,12 @@ def select_most_likely_errors( When multiple groups have the same minimum error count (a tie), returns all tied groups to indicate ambiguity to the user. + Before tie detection, sibling-branch `union_tag_not_found` noise is + suppressed for any item that already matched a concrete type (see + `_suppress_sibling_tag_noise`), so a clean match is not falsely + reported as ambiguous. Items whose only errors are `union_tag_not_found` + are left intact. + For heterogeneous collections, returns ALL errors since different items may have different intended types. @@ -219,6 +303,11 @@ def select_most_likely_errors( return filtered_errors, False, True, _item_types + # Suppress sibling-branch discriminator noise before tie detection. + error_groups = _suppress_sibling_tag_noise(error_groups) + if not error_groups: + return [], False, is_heterogeneous, _item_types + # Find the minimum error count min_error_count = min(len(errors) for errors in error_groups.values()) diff --git a/packages/overture-schema-cli/tests/test_cli_functions.py b/packages/overture-schema-cli/tests/test_cli_functions.py index 5a3139720..601e6f3b6 100644 --- a/packages/overture-schema-cli/tests/test_cli_functions.py +++ b/packages/overture-schema-cli/tests/test_cli_functions.py @@ -3,14 +3,23 @@ import io import json from pathlib import Path +from typing import Literal, cast import pytest import yaml from click.exceptions import UsageError from conftest import build_feature -from overture.schema.cli.commands import load_input, perform_validation, resolve_types +from overture.schema.cli.commands import ( + _best_fit_model, + _revalidate_undiscriminatable_items, + load_input, + perform_validation, + resolve_types, +) +from overture.schema.cli.type_analysis import get_item_index +from overture.schema.cli.types import ValidationErrorDict from overture.schema.system.discovery import TagSelector -from pydantic import ValidationError +from pydantic import BaseModel, ValidationError class TestLoadInput: @@ -266,3 +275,134 @@ def test_perform_validation_with_different_themes(self) -> None: places_type = resolve_types(TagSelector(include_any=("overture:theme=places",))) with pytest.raises(ValidationError): perform_validation(data, places_type) + + +class _Building(BaseModel): + type: Literal["building"] + id: str + + +class _Place(BaseModel): + type: Literal["place"] + id: str + name: str + + +class _Sources(BaseModel): + """A plain, non-discriminated model (no literal `type` field).""" + + datasets: list[str] + + +class TestRevalidateUndiscriminatableItems: + """Tests for the best-fit re-validation of undiscriminatable list items. + + Covers an all-discriminated union (a typeless item yields only + `union_tag_not_found` and must be re-validated to surface field errors) and + a union that includes a plain, non-discriminated member (the typeless item + already has concrete field errors, so the helper must be a no-op). + """ + + def test_all_discriminated_union_substitutes_best_fit_field_errors( + self, + ) -> None: + """All-discriminated union: typeless item (only union_tag_not_found) -> field errors. + + With an all-discriminated union, a typeless item produces only + `union_tag_not_found`. The helper must re-validate it against the + candidates, pick the best fit (fewest errors -> `_Building`, which + only needs `type`, over `_Place` which needs `type` and `name`), + and re-home concrete field errors under the item index. + """ + errors = cast( + list[ValidationErrorDict], + [ + { + "loc": (0, "tagged-union[type]"), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + ], + ) + typeless_item = {"id": "x"} # missing the 'type' discriminator + + augmented, best_fit_types = _revalidate_undiscriminatable_items( + errors, [typeless_item], (_Building, _Place) + ) + + # Best fit is the fewest-error candidate. + assert best_fit_types == {0: _Building} + + item0 = [e for e in augmented if get_item_index(e["loc"]) == 0] + assert item0, "item 0 should still have errors" + # The opaque tag-not-found noise is gone; concrete field errors remain. + assert all(e["type"] != "union_tag_not_found" for e in item0) + assert any(e["type"] == "missing" for e in item0) + # Errors are re-homed under the item index. + assert all(e["loc"][0] == 0 for e in item0) + + def test_noop_when_concrete_errors_coexist_with_tag_not_found(self) -> None: + """A plain member yields concrete errors, so the helper is a no-op. + + When the union includes a plain (non-discriminated) member, a typeless + item also matches that member and produces concrete `missing` errors + alongside the discriminated branch's `union_tag_not_found`. Because the + item already has a concrete error, the helper must leave the errors + untouched and infer no best-fit type. + """ + errors = cast( + list[ValidationErrorDict], + [ + { + "loc": (0, "tagged-union[type]"), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + { + "loc": (0, "_Sources", "datasets"), + "msg": "Field required", + "type": "missing", + }, + ], + ) + + augmented, best_fit_types = _revalidate_undiscriminatable_items( + errors, [{"id": "x"}], (_Building,) + ) + + assert best_fit_types == {}, "helper must not fire when concrete errors exist" + assert augmented == errors, "errors must be left unchanged" + + def test_noop_when_original_data_is_not_a_list(self) -> None: + """Graceful degradation: without list data there is nothing to re-home.""" + errors = cast( + list[ValidationErrorDict], + [ + { + "loc": ("tagged-union[type]",), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + ], + ) + + augmented, best_fit_types = _revalidate_undiscriminatable_items( + errors, None, (_Building,) + ) + + assert best_fit_types == {} + assert augmented == errors + + +class TestBestFitModel: + """Tests for the `_best_fit_model` best-fit candidate selection helper.""" + + def test_best_fit_model_prefers_fewest_errors(self) -> None: + """_best_fit_model returns the candidate needing the fewest changes.""" + result = _best_fit_model({"id": "x"}, (_Place, _Building)) + + assert result is not None + model, errs = result + # _Building needs only 'type'; _Place needs 'type' and 'name'. + assert model is _Building + assert len(errs) < 2 diff --git a/packages/overture-schema-cli/tests/test_error_formatting.py b/packages/overture-schema-cli/tests/test_error_formatting.py index 168d9d53c..b87aa0f15 100644 --- a/packages/overture-schema-cli/tests/test_error_formatting.py +++ b/packages/overture-schema-cli/tests/test_error_formatting.py @@ -1,7 +1,7 @@ """Tests for error formatting and grouping logic.""" from io import StringIO -from typing import Annotated, Literal +from typing import Annotated, Literal, cast from unittest.mock import patch import pytest @@ -13,6 +13,7 @@ select_most_likely_errors, ) from overture.schema.cli.type_analysis import introspect_union +from overture.schema.cli.types import ErrorLocation, ValidationErrorDict from pydantic import BaseModel, Field, TypeAdapter, ValidationError from rich.console import Console @@ -20,6 +21,133 @@ class TestErrorGrouping: """Tests for error grouping and selection logic.""" + def test_sibling_tag_not_found_noise_does_not_cause_false_tie(self) -> None: + """Sibling-branch union_tag_not_found noise must not tie with a real match. + + When validating a list against a union that mixes a tagged-union with + another discriminated member (e.g. Segment), each item that cleanly + matches one branch still emits a `union_tag_not_found` error from the + sibling branch. Those empty-path `()` errors must not form a group that + ties with the real discriminator group and raises a false "Ambiguous" + warning. Regression guard for a union-shape invariant: error grouping + must not depend on how many non-discriminated members the union + contains. + """ + # Two list items, each with one real 'missing' error (building) plus one + # sibling-branch union_tag_not_found error (empty discriminator path). + error_groups = cast( + dict[ErrorLocation, list[ValidationErrorDict]], + { + ("building",): [ + { + "loc": (0, "tagged-union[type]", "building", "id"), + "msg": "Field required", + "type": "missing", + }, + { + "loc": (1, "tagged-union[type]", "building", "id"), + "msg": "Field required", + "type": "missing", + }, + ], + (): [ + { + "loc": (0, "tagged-union[subtype]"), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + { + "loc": (1, "tagged-union[subtype]"), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + ], + }, + ) + + selected, is_tied, is_heterogeneous, _ = select_most_likely_errors(error_groups) + + assert not is_tied, "Sibling tag-not-found noise must not create a tie" + assert not is_heterogeneous + # Only the real building errors survive; the () noise group is dropped. + assert len(selected) == 2 + assert all(error["type"] == "missing" for error in selected) + + def test_only_tag_not_found_errors_are_preserved(self) -> None: + """An item whose ONLY errors are union_tag_not_found must be preserved. + + The sibling-noise suppression must not strip tag-not-found errors from a + genuinely undiscriminatable item (one with no concrete match), otherwise + there would be nothing to report for it. + """ + error_groups = cast( + dict[ErrorLocation, list[ValidationErrorDict]], + { + (): [ + { + "loc": (2, "tagged-union[type]"), + "msg": "Unable to extract tag", + "type": "union_tag_not_found", + }, + ], + }, + ) + + selected, is_tied, _, _ = select_most_likely_errors(error_groups) + + assert not is_tied + assert len(selected) == 1 + assert selected[0]["type"] == "union_tag_not_found" + + def test_genuine_tie_between_real_types_is_still_reported(self) -> None: + """A genuine tie between two real candidate types must still be reported. + + When the union contains a plain (non-discriminated) member alongside + discriminated types, data can match two real types equally well (each + with the same number of concrete `missing` errors), producing + legitimate ambiguity. The sibling-noise suppression only removes + `union_tag_not_found` errors, so a genuine tie built from concrete + errors must remain a tie. Guards against the suppression logic being + broadened to swallow real ambiguity. + """ + # Two real candidate groups, equal concrete 'missing' counts, no noise. + error_groups = cast( + dict[ErrorLocation, list[ValidationErrorDict]], + { + ("building",): [ + { + "loc": ("building", "id"), + "msg": "Field required", + "type": "missing", + }, + { + "loc": ("building", "height"), + "msg": "Field required", + "type": "missing", + }, + ], + ("Sources",): [ + { + "loc": ("Sources", "datasets"), + "msg": "Field required", + "type": "missing", + }, + { + "loc": ("Sources", "license_priority"), + "msg": "Field required", + "type": "missing", + }, + ], + }, + ) + + selected, is_tied, is_heterogeneous, _ = select_most_likely_errors(error_groups) + + assert is_tied, "A genuine tie between two real types must still be reported" + assert not is_heterogeneous + # All errors from both tied groups are returned. + assert len(selected) == 4 + def test_ambiguous_data_shows_most_likely_errors( self, cli_runner: CliRunner ) -> None: From 2ac5e36ed3a1d4ab87f5aba483994bee8701a123 Mon Sep 17 00:00:00 2001 From: schapper Date: Fri, 31 Jul 2026 16:00:20 -0700 Subject: [PATCH 3/3] Fix last doc format errors, enable `docformat` check in `make check` Signed-off-by: schapper --- Makefile | 8 +++++--- .../src/overture/schema/codegen/pyspark/check_builder.py | 2 +- .../overture/schema/codegen/pyspark/test_data/scaffold.py | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index cd1b91497..33ce34306 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: default uv-sync clean-pyspark generate-pyspark check test-all test test-only docformat doctest doctest-only mypy mypy-only lint-only update-baselines +.PHONY: default uv-sync clean-pyspark generate-pyspark check test-all test test-only docformat docformat-only doctest doctest-only mypy mypy-only lint-only update-baselines TESTMON ?= --testmon @@ -23,7 +23,7 @@ generate-pyspark: uv-sync clean-pyspark @uv run ruff format --quiet $(PYSPARK_EXPRESSIONS) $(PYSPARK_GENERATED_TESTS) check: uv-sync generate-pyspark - @$(MAKE) -j test-only doctest-only lint-only mypy-only + @$(MAKE) -j test-only docformat-only doctest-only lint-only mypy-only # test-all is the unconditional full run -- testmon-independent, unlike the # incremental test/test-only targets -- so data-only changes (golden JSON, @@ -42,7 +42,9 @@ test-only: coverage: uv-sync @uv run pytest packages/ --cov overture.schema --cov-report=term --cov-report=html && open htmlcov/index.html -docformat: +docformat: uv-sync docformat-only + +docformat-only: @find packages/*/src -name "*.py" -type f -not -name "__*" \ | xargs uv run pydocstyle --convention=numpy --add-ignore=D102,D105,D200,D205,D400 diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/check_builder.py b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/check_builder.py index 4a35a09d1..b3199423f 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/check_builder.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/check_builder.py @@ -636,7 +636,7 @@ def _guard_struct_nested_variant_fields(prefix: FieldPath, name: str) -> None: def _iteration_depth(path: FieldPath) -> int: - """Number of iteration frames (`Array`/`Map` segments) in *path*. + """Return the number of iteration frames (`Array`/`Map` segments) in *path*. Each iterating segment -- named or anonymous -- is one lambda frame in the renderer's fold, so the count is the depth at which the innermost diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/test_data/scaffold.py b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/test_data/scaffold.py index a911fb4b7..77d0f83db 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/test_data/scaffold.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/pyspark/test_data/scaffold.py @@ -74,7 +74,7 @@ class _ElementDiscriminator: def _is_anonymous_iter(seg: FieldSegment) -> bool: - """True when *seg* iterates a container nested directly inside another. + """Return True when *seg* iterates a container nested directly inside another. In a run of nested containers the first takes the field's name; each further level is *anonymous*, because no field name introduces it -- the