From 139e24a9516bf2680e23a9708e1b7927ac9ef42a Mon Sep 17 00:00:00 2001 From: jh-RLI Date: Thu, 16 Jul 2026 12:56:59 +0200 Subject: [PATCH 1/2] fix(conversion): coerce v1.5 string primaryKey to list; drop empty foreignKeys stub v1.5/v1.6 OEMetadata stores `primaryKey` as a comma-separated string (e.g. "id, scn_name"). The v1.6->v2 populator iterated it with enumerate(), splitting the string character by character into ['i','d',',',' ',...] and emitting an invalid primaryKey. It also leaked the v2 template placeholder into `foreignKeys` when the source declared none. - primaryKey: coerce a string to a list (split on commas, strip); map the "none" sentinel and empty values to []; pass lists through unchanged. - foreignKeys: emit [] when the source has none; fix the always-true isinstance(fk, object) guard to isinstance(fk, dict). - add 8 regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/omi/conversions/v160_to_v20.py | 45 +++++++++++---- tests/test_conversion_primarykey.py | 89 +++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 tests/test_conversion_primarykey.py diff --git a/src/omi/conversions/v160_to_v20.py b/src/omi/conversions/v160_to_v20.py index d416bbb8..e6172c0b 100644 --- a/src/omi/conversions/v160_to_v20.py +++ b/src/omi/conversions/v160_to_v20.py @@ -304,23 +304,48 @@ def rename_path_to_id(annotation_object: dict) -> dict: def ___v2_populate_schema_primary_keys(resource_v2: dict, resource: dict) -> None: - """Populate schema fields in resource_v2 from resource in v1.6.""" - for i_pk, pk in enumerate(resource.get("schema", {}).get("primaryKey", []) or []): - if i_pk >= len(resource_v2["schema"]["primaryKey"]): - resource_v2["schema"]["primaryKey"].append(deepcopy(resource_v2["schema"]["primaryKey"][0])) + """ + Populate schema.primaryKey in resource_v2 from the v1.6 resource. + + v1.5/v1.6 may store ``primaryKey`` as a comma-separated string + (e.g. ``"id, scn_name"``) or as a list; OEMetadata v2 requires a list of + column names. A string is split on commas and whitespace-stripped; the + ``"none"`` sentinel and empty values become an empty list. Iterating the + string directly (the previous behaviour) split it into individual + characters, so this coercion must happen before any iteration. + """ + raw_pk = resource.get("schema", {}).get("primaryKey", []) or [] - if isinstance(pk, str): - resource_v2["schema"]["primaryKey"].pop() - resource_v2["schema"]["primaryKey"].append(pk) + if isinstance(raw_pk, str): + if raw_pk.strip().lower() in ("", "none"): + primary_keys: list = [] + else: + primary_keys = [part.strip() for part in raw_pk.split(",") if part.strip()] + else: + primary_keys = [pk for pk in raw_pk if pk not in (None, "")] + + resource_v2["schema"]["primaryKey"] = primary_keys def ___v2_populate_schema_foreign_keys(resource_v2: dict, resource: dict) -> None: - """Populate schema fields in resource_v2 from resource in v1.6.""" - for i_fk, fk in enumerate(resource.get("schema", {}).get("foreignKeys", [])): + """ + Populate schema.foreignKeys in resource_v2 from the v1.6 resource. + + When the source declares no foreign keys, emit an empty list rather than + leaking the v2 template placeholder stub (an empty ``{fields, reference}`` + entry) into the output. + """ + source_fks = resource.get("schema", {}).get("foreignKeys", []) or [] + + if not source_fks: + resource_v2["schema"]["foreignKeys"] = [] + return + + for i_fk, fk in enumerate(source_fks): if i_fk >= len(resource_v2["schema"]["foreignKeys"]): resource_v2["schema"]["foreignKeys"].append(deepcopy(resource_v2["schema"]["foreignKeys"][0])) - if isinstance(fk, object): + if isinstance(fk, dict): resource_v2["schema"]["foreignKeys"][i_fk].update( (k, resource["schema"]["foreignKeys"][i_fk][k]) for k in resource_v2["schema"]["foreignKeys"][i_fk].keys() diff --git a/tests/test_conversion_primarykey.py b/tests/test_conversion_primarykey.py new file mode 100644 index 00000000..3fdf2de7 --- /dev/null +++ b/tests/test_conversion_primarykey.py @@ -0,0 +1,89 @@ +""" +Regression tests: v1.5/v1.6 -> v2 primaryKey / foreignKeys coercion. + +A v1.5 ``primaryKey`` stored as a comma-separated string (e.g. "id, scn_name") +must convert to a list of column names, not be iterated character by character. +A resource without foreign keys must yield ``foreignKeys: []``, not the v2 +template placeholder stub. +""" + +from __future__ import annotations + +from omi.conversions.v160_to_v20 import ( + ___v2_populate_schema_foreign_keys as populate_fk, +) +from omi.conversions.v160_to_v20 import ( + ___v2_populate_schema_primary_keys as populate_pk, +) + + +def _resource_v2_template() -> dict: + """Return a resource_v2 seeded with the v2 spec placeholders (as during conversion).""" + return { + "schema": { + "primaryKey": [""], + "foreignKeys": [{"fields": [""], "reference": {"resource": "", "fields": [""]}}], + }, + } + + +def test_comma_separated_string_pk_splits_into_columns() -> None: + """A comma-separated string primaryKey splits into one column per name.""" + rv2 = _resource_v2_template() + populate_pk(rv2, {"schema": {"primaryKey": "id, scn_name"}}) + assert rv2["schema"]["primaryKey"] == ["id", "scn_name"] + + +def test_single_column_string_pk() -> None: + """A single-column string primaryKey becomes a one-element list, not chars.""" + rv2 = _resource_v2_template() + populate_pk(rv2, {"schema": {"primaryKey": "bus_id"}}) + assert rv2["schema"]["primaryKey"] == ["bus_id"] + + +def test_none_sentinel_pk_becomes_empty_list() -> None: + """The v1.5 'none' sentinel maps to an empty primaryKey list.""" + rv2 = _resource_v2_template() + populate_pk(rv2, {"schema": {"primaryKey": "none"}}) + assert rv2["schema"]["primaryKey"] == [] + + +def test_list_pk_passes_through() -> None: + """An already-list primaryKey is preserved unchanged.""" + rv2 = _resource_v2_template() + populate_pk(rv2, {"schema": {"primaryKey": ["id", "scn_name"]}}) + assert rv2["schema"]["primaryKey"] == ["id", "scn_name"] + + +def test_missing_pk_becomes_empty_list() -> None: + """A resource with no primaryKey yields an empty list.""" + rv2 = _resource_v2_template() + populate_pk(rv2, {"schema": {}}) + assert rv2["schema"]["primaryKey"] == [] + + +def test_empty_foreign_keys_yields_empty_list_not_stub() -> None: + """An empty source foreignKeys list yields [], not the template stub.""" + rv2 = _resource_v2_template() + populate_fk(rv2, {"schema": {"foreignKeys": []}}) + assert rv2["schema"]["foreignKeys"] == [] + + +def test_absent_foreign_keys_yields_empty_list_not_stub() -> None: + """A missing foreignKeys key yields [], not the template stub.""" + rv2 = _resource_v2_template() + populate_fk(rv2, {"schema": {}}) + assert rv2["schema"]["foreignKeys"] == [] + + +def test_populated_foreign_keys_are_carried_over() -> None: + """A populated source foreignKey is carried into the v2 output.""" + rv2 = _resource_v2_template() + populate_fk( + rv2, + {"schema": {"foreignKeys": [{"fields": ["bus_id"], "reference": {"resource": "grid.bus", "fields": ["id"]}}]}}, + ) + fks = rv2["schema"]["foreignKeys"] + assert len(fks) == 1 + assert fks[0]["fields"] == ["bus_id"] + assert fks[0]["reference"]["resource"] == "grid.bus" From c58d0c1991a84d6923716e75e6269e15b0a258e1 Mon Sep 17 00:00:00 2001 From: jh-RLI Date: Thu, 16 Jul 2026 12:57:00 +0200 Subject: [PATCH 2/2] feat(creation): add-source, coverage report, and DB merge-update helpers Generic, project-agnostic capabilities supporting external metadata stores (e.g. eGon-data) with the OMI-first split: - sources.py: build external / internal-cross-reference source entries and append them to a resource's `sources` list non-destructively (deduped). - coverage.py: compare a caller-supplied expected resource-name list against a split-YAML store -> missing / skeleton / complete / orphan. - init.py: update_resource_from_db_skeleton / update_resource_yaml_from_db -- reusable non-destructive merge of a DB-inspection skeleton into a resource. - cli.py: `omi add-source` and `omi coverage` commands. - tests for all three. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/omi/cli.py | 97 +++++++++++++++++ src/omi/creation/coverage.py | 200 +++++++++++++++++++++++++++++++++++ src/omi/creation/init.py | 95 ++++++++++++++++- src/omi/creation/sources.py | 145 +++++++++++++++++++++++++ tests/test_coverage.py | 123 +++++++++++++++++++++ tests/test_merge_update.py | 57 ++++++++++ tests/test_sources.py | 97 +++++++++++++++++ 7 files changed, 813 insertions(+), 1 deletion(-) create mode 100644 src/omi/creation/coverage.py create mode 100644 src/omi/creation/sources.py create mode 100644 tests/test_coverage.py create mode 100644 tests/test_merge_update.py create mode 100644 tests/test_sources.py diff --git a/src/omi/cli.py b/src/omi/cli.py index cb03111a..88819a9c 100644 --- a/src/omi/cli.py +++ b/src/omi/cli.py @@ -623,6 +623,103 @@ def inspect_db_drift_cmd( # noqa: PLR0913 click.secho(f"✓ Automatically updated resource YAML: {resource_path.name}", fg="green") +@grp.command("add-source") +@click.argument("resource_yaml", type=click.Path(exists=True, dir_okay=False, path_type=Path)) +@click.option( + "--kind", + type=click.Choice(["external", "internal"]), + default="external", + show_default=True, + help="external: literature/other database; internal: another table in this project.", +) +@click.option("--title", default=None, help="Source title (required for --kind external).") +@click.option("--table", "ref_table", default=None, help="Referenced table name (required for --kind internal).") +@click.option("--path", "src_path", default="", help="URL or reference path of the source.") +@click.option("--description", default="", help="Human-readable source description.") +def add_source_cmd( # noqa: PLR0913 + resource_yaml: Path, + kind: str, + title: Optional[str], + ref_table: Optional[str], + src_path: str, + description: str, +) -> None: + """Append a provenance source to a resource YAML (non-destructive, de-duplicated).""" + from omi.creation.sources import ( + add_source_to_resource_file, + build_external_source, + build_internal_source, + ) + + if kind == "internal": + if not ref_table: + msg = "--table is required for --kind internal" + raise click.UsageError(msg) + source = build_internal_source(ref_table, title=title, description=description, path=src_path) + else: + if not title: + msg = "--title is required for --kind external" + raise click.UsageError(msg) + source = build_external_source(title, description=description, path=src_path) + + added = add_source_to_resource_file(resource_yaml, source) + if added: + click.secho(f"✓ Added {kind} source to {resource_yaml}", fg="green") + else: + click.secho(f"= Equivalent source already present in {resource_yaml}; unchanged", fg="yellow") + + +@grp.command("coverage") +@click.argument("base_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) +@click.option( + "--expected-file", + required=True, + type=click.Path(exists=True, dir_okay=False, path_type=Path), + help="File listing expected resource names, one per line ('#' comments allowed).", +) +@click.option("--dataset-id", "dataset_ids", multiple=True, help="Restrict scan to these dataset ids (repeatable).") +@click.option("--strict", is_flag=True, help="Exit non-zero if any expected resource is missing.") +@click.option( + "--no-field-descriptions", + is_flag=True, + help="Do not require per-field descriptions for a resource to count as complete.", +) +def coverage_cmd( + base_dir: Path, + expected_file: Path, + dataset_ids: tuple[str, ...], + *, + strict: bool, + no_field_descriptions: bool, +) -> None: + """Report documented/skeleton/missing/orphan coverage against an expected resource list.""" + from omi.creation.coverage import coverage_report + + expected = [ + line.strip() + for line in expected_file.read_text(encoding="utf-8").splitlines() + if line.strip() and not line.strip().startswith("#") + ] + report = coverage_report( + base_dir, + expected, + dataset_ids=dataset_ids or None, + require_field_descriptions=not no_field_descriptions, + ) + + click.echo(report.summary()) + for name in report.missing: + click.secho(f"[missing] {name}", fg="red") + for state in report.skeleton: + detail = "; ".join(state.reasons[:3]) + click.secho(f"[skeleton] {state.name} ({detail})", fg="yellow") + for name in report.orphans: + click.secho(f"[orphan] {name}", fg="cyan") + + if strict and not report.ok: + raise click.Abort + + # Keep CommandCollection for backwards compatibility with your entry point cli = click.CommandCollection(sources=[grp, init, inspect]) diff --git a/src/omi/creation/coverage.py b/src/omi/creation/coverage.py new file mode 100644 index 00000000..846ed987 --- /dev/null +++ b/src/omi/creation/coverage.py @@ -0,0 +1,200 @@ +# omi/creation/coverage.py +""" +Coverage reporting for an OEMetadata split-YAML store. + +Given an *expected* set of resource names (the tables that should be +documented -- supplied by the caller, e.g. derived from a pipeline's declared +outputs), report which are: + +- **missing** -- expected but no resource YAML exists (hard gap), +- **skeleton** -- a YAML exists but required human fields are still blank or + carry scaffolding placeholders (soft gap / warning), +- **complete** -- fully documented, + +plus **orphans** -- resources present in the store that nobody expects. + +The expected list is caller-supplied so this stays project-agnostic: OMI does +not know about any particular pipeline's output declarations; the caller +computes the list and passes resource names in. +""" +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path +from typing import TYPE_CHECKING, Optional, Union + +from .utils import discover_dataset_ids, discover_paths, load_yaml + +if TYPE_CHECKING: + from collections.abc import Iterable + +Json = dict[str, object] + +# a filled field must not still carry a scaffolding placeholder +_PLACEHOLDER_MARKERS: tuple[str, ...] = ("TODO", "WILL_BE_SET_AT_PUBLICATION") +# resource-level fields a human must fill for a resource to count as complete +_DEFAULT_REQUIRED_RESOURCE_FIELDS: tuple[str, ...] = ("title", "description") + + +@dataclass +class ResourceState: + """Coverage state of a single expected resource.""" + + name: str + state: str # "missing" | "skeleton" | "complete" + path: Optional[Path] = None + reasons: list[str] = field(default_factory=list) + + +@dataclass +class CoverageReport: + """Result of comparing an expected resource list against the store.""" + + missing: list[str] = field(default_factory=list) # expected, no YAML + skeleton: list[ResourceState] = field(default_factory=list) + complete: list[str] = field(default_factory=list) + orphans: list[str] = field(default_factory=list) # in store, not expected + states: dict[str, ResourceState] = field(default_factory=dict) + + @property + def ok(self) -> bool: + """True when nothing expected is missing (skeletons are warnings).""" + return not self.missing + + def summary(self) -> str: + """One-line human-readable tally.""" + return ( + f"complete={len(self.complete)} " + f"skeleton={len(self.skeleton)} " + f"missing={len(self.missing)} " + f"orphans={len(self.orphans)}" + ) + + +def _is_blank(value: object) -> bool: + return value in (None, "", [], {}) + + +def _has_placeholder(value: object) -> bool: + return isinstance(value, str) and any(m in value for m in _PLACEHOLDER_MARKERS) + + +def resource_completeness( + resource: Json, + *, + required_fields: Iterable[str] = _DEFAULT_REQUIRED_RESOURCE_FIELDS, + require_field_descriptions: bool = True, +) -> tuple[str, list[str]]: + """ + Classify a resource dict as ``"complete"`` or ``"skeleton"``. + + A resource is *complete* when every required resource-level field is + non-empty and free of scaffolding placeholders and -- when + ``require_field_descriptions`` is set -- every schema field carries a + placeholder-free description. Otherwise it is a *skeleton*. + + Returns + ------- + (state, reasons) + ``reasons`` lists every unmet condition (empty when complete). + """ + reasons: list[str] = [] + + for name in required_fields: + value = resource.get(name) + if _is_blank(value): + reasons.append(f"'{name}' is empty") + elif _has_placeholder(value): + reasons.append(f"'{name}' still has a placeholder") + + if require_field_descriptions: + schema = resource.get("schema") + fields = schema.get("fields") if isinstance(schema, dict) else None + if isinstance(fields, list): + for fld in fields: + if not isinstance(fld, dict): + continue + col = fld.get("name", "?") + desc = fld.get("description") + if _is_blank(desc): + reasons.append(f"field '{col}' has no description") + elif _has_placeholder(desc): + reasons.append(f"field '{col}' description is a placeholder") + + return ("complete" if not reasons else "skeleton", reasons) + + +def index_store_resources( + base_dir: Union[str, Path], + *, + dataset_ids: Optional[Iterable[str]] = None, +) -> dict[str, Path]: + """ + Map every resource *name* present in the store to its YAML path. + + Scans the given dataset ids (or all discovered ids) and reads each + resource YAML's ``name`` field. A name defined in more than one dataset + resolves to the last one scanned. + """ + base = Path(base_dir) + ids = list(dataset_ids) if dataset_ids is not None else discover_dataset_ids(base) + index: dict[str, Path] = {} + for ds_id in ids: + _, _, resource_paths = discover_paths(base, ds_id) + for path in resource_paths: + doc = load_yaml(path) + name = doc.get("name") + if isinstance(name, str) and name: + index[name] = path + return index + + +def coverage_report( + base_dir: Union[str, Path], + expected: Iterable[str], + *, + dataset_ids: Optional[Iterable[str]] = None, + required_fields: Iterable[str] = _DEFAULT_REQUIRED_RESOURCE_FIELDS, + require_field_descriptions: bool = True, +) -> CoverageReport: + """ + Compare an expected resource-name list against the store. + + Parameters + ---------- + base_dir : + Path to the metadata store (contains ``datasets/`` and ``resources/``). + expected : + Resource names that *should* be documented (e.g. ``schema.table``). + dataset_ids : + Restrict the scan to these dataset ids (default: all discovered). + required_fields, require_field_descriptions : + Passed through to :func:`resource_completeness` to tune the + skeleton/complete threshold. + """ + expected_names = list(dict.fromkeys(expected)) # dedupe, preserve order + store = index_store_resources(base_dir, dataset_ids=dataset_ids) + required = tuple(required_fields) + + report = CoverageReport() + for name in expected_names: + path = store.get(name) + if path is None: + state = ResourceState(name=name, state="missing") + report.missing.append(name) + else: + level, reasons = resource_completeness( + load_yaml(path), + required_fields=required, + require_field_descriptions=require_field_descriptions, + ) + state = ResourceState(name=name, state=level, path=path, reasons=reasons) + if level == "complete": + report.complete.append(name) + else: + report.skeleton.append(state) + report.states[name] = state + + expected_set = set(expected_names) + report.orphans = sorted(name for name in store if name not in expected_set) + return report diff --git a/src/omi/creation/init.py b/src/omi/creation/init.py index a2aa8915..9218802e 100644 --- a/src/omi/creation/init.py +++ b/src/omi/creation/init.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +from copy import deepcopy from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING, Union @@ -15,7 +16,8 @@ import yaml from omi.base import MetadataError, get_metadata_specification -from omi.inspection import InspectionError, infer_metadata +from omi.creation.builder import MetadataBuilder +from omi.inspection import InspectionError, infer_metadata, inspect_db_table from .utils import ( collect_common_resource_fields, @@ -475,3 +477,94 @@ def add_resource_from_oem_metadata( # noqa: PLR0913 dump_yaml(out_path, out) return out_path + + +# --------------------------------------------------------------------------- +# Non-destructive merge-update of a resource from a DB-inspection skeleton +# --------------------------------------------------------------------------- + + +def update_resource_from_db_skeleton( + resource: dict, + db_skeleton: dict, + *, + strict: bool = False, +) -> tuple[dict, dict]: + """ + Merge a DB-inspection skeleton into an existing resource, non-destructively. + + Human-authored content in ``resource`` (field descriptions, units, titles) + is preserved; the database supplies structural truth (column presence, + types, nullability). Columns present in the DB but absent from the YAML are + added with a ``TODO`` description placeholder; columns present in the YAML + but absent from the DB are kept and reported as drift. + + Parameters + ---------- + resource : + An OEMetadata resource dict (as loaded from a ``.resource.yaml``). + db_skeleton : + The resource skeleton produced by :func:`omi.inspection.inspect_db_table`. + strict : + If True, raise ``SchemaDriftError`` when columns are missing on either + side instead of merging and reporting. + + Returns + ------- + (updated_resource, drift_report) + ``updated_resource`` is a new dict (the input is not mutated); + ``drift_report`` has keys ``missing_in_yaml``, ``missing_in_db`` and + ``type_mismatches``. + """ + builder = MetadataBuilder({"resources": [deepcopy(resource)]}) + report = builder.resource(0).merge_and_diff_db_schema(db_skeleton, strict=strict) + updated = builder.build(validate_policy="skip")["resources"][0] + return dict(updated), report + + +def update_resource_yaml_from_db( # noqa: PLR0913 + base_dir: Union[str, Path], + dataset_id: str, + resource_name: str, + engine_or_url: object, + *, + schema_name: str | None = None, + table_name: str | None = None, + strict: bool = False, +) -> tuple[Path, dict]: + """ + Update a resource YAML in place from a live database table. + + Loads ``resources//.resource.yaml`` (where + ``safe_name`` is ``resource_name`` with ``.`` replaced by ``_``), inspects + the corresponding DB table, merges the schema non-destructively via + :func:`update_resource_from_db_skeleton`, and writes the file back. + + ``schema_name`` / ``table_name`` default to the two halves of a dotted + ``resource_name`` (``schema.table``). + + Returns + ------- + (resource_path, drift_report) + """ + base_dir = Path(base_dir) + safe_name = resource_name.replace(".", "_") + resource_path = base_dir / "resources" / dataset_id / f"{safe_name}.resource.yaml" + if not resource_path.exists(): + raise FileNotFoundError(f"Resource YAML not found: {resource_path}") + + if schema_name is None or table_name is None: + parts = resource_name.split(".") + expected_parts = 2 + if len(parts) == expected_parts: + schema_name = schema_name or parts[0] + table_name = table_name or parts[1] + else: + table_name = table_name or resource_name + + db_skeleton = inspect_db_table(engine_or_url, schema_name or "", table_name or "") + + resource = load_yaml(resource_path) + updated, report = update_resource_from_db_skeleton(resource, db_skeleton, strict=strict) + dump_yaml(resource_path, updated) + return resource_path, report diff --git a/src/omi/creation/sources.py b/src/omi/creation/sources.py new file mode 100644 index 00000000..19ecfd48 --- /dev/null +++ b/src/omi/creation/sources.py @@ -0,0 +1,145 @@ +# omi/creation/sources.py +""" +Attach provenance sources to OEMetadata resources. + +Two kinds of source are supported, both living in the OEMetadata v2 ``sources`` +slot on a resource: + +- *external* sources -- literature, databases or portals outside the dataset + (e.g. MaStR, OSM, a Destatis publication). Recorded with descriptive fields + a human fills in. +- *internal* cross-references -- another table produced within the same + project whose own metadata carries the full provenance. Recorded as a + lightweight ``sources`` entry so the consuming resource declares where its + input came from without duplicating that table's metadata. + +Both constructors return plain, spec-shaped ``dict`` source entries; +:func:`add_source_to_resource` appends them without creating duplicates. The +functions are project-agnostic -- callers decide which tables are internal vs. +external. +""" +from __future__ import annotations + +from copy import deepcopy +from pathlib import Path +from typing import Optional, Union + +from .utils import dump_yaml, load_yaml + +Json = dict[str, object] + +# keys that decide whether two source entries describe the same thing +_SOURCE_DEDUPE_KEYS: tuple[str, ...] = ("title", "path") + + +def build_external_source( # noqa: PLR0913 + title: str, + *, + description: str = "", + path: str = "", + authors: Optional[list[str]] = None, + publication_year: Optional[int] = None, + licenses: Optional[list[Json]] = None, +) -> Json: + """ + Build an external-source entry for a resource's ``sources`` list. + + Only ``title`` is required; the remaining descriptive fields default to + empty stubs for a human to complete later. + """ + return { + "title": title, + "description": description, + "path": path, + "authors": list(authors) if authors else [], + "publicationYear": publication_year, + "sourceLicenses": list(licenses) if licenses else [], + } + + +def build_internal_source( + table: str, + *, + title: Optional[str] = None, + description: str = "", + path: str = "", +) -> Json: + """ + Build an internal cross-reference entry pointing at another table. + + ``table`` is the referenced resource name (e.g. ``"grid.egon_etrago_bus"``). + The full provenance lives in that table's own metadata; this entry only + records the dependency. ``path`` may point at the referenced metadata (an + ``@id`` / URL) once known. + + The result uses only spec ``sources`` keys so it stays valid against the + OEMetadata schema; the internal nature is conveyed by the content. + """ + ref_title = title or f"Internal table: {table}" + ref_desc = description or ( + f"Derived from the internal table '{table}'. See that table's own metadata for full provenance." + ) + return { + "title": ref_title, + "description": ref_desc, + "path": path, + "authors": [], + "publicationYear": None, + "sourceLicenses": [], + } + + +def add_source_to_resource( + resource: Json, + source: Json, + *, + dedupe_keys: tuple[str, ...] = _SOURCE_DEDUPE_KEYS, +) -> tuple[Json, bool]: + """ + Return a copy of ``resource`` with ``source`` appended to its ``sources``. + + An equivalent source (matching on ``dedupe_keys``, default title + path) + is not added twice. The input ``resource`` is never mutated in place. + + Returns + ------- + (resource, added) + The updated resource copy and whether a new source was appended. + """ + result = deepcopy(resource) + existing = result.get("sources") + existing = list(existing) if isinstance(existing, list) else [] + + def sig(s: object) -> tuple: + if not isinstance(s, dict): + return (repr(s),) + return tuple(s.get(k) for k in dedupe_keys) + + target = sig(source) + if any(sig(s) == target for s in existing): + result["sources"] = existing + return result, False + + existing.append(deepcopy(source)) + result["sources"] = existing + return result, True + + +def add_source_to_resource_file( + resource_path: Union[str, Path], + source: Json, + *, + dedupe_keys: tuple[str, ...] = _SOURCE_DEDUPE_KEYS, +) -> bool: + """ + Load a resource YAML, add ``source`` non-destructively, and write it back. + + Returns True if a new source was written, False if an equivalent one was + already present (file left untouched in that case). + """ + path = Path(resource_path) + resource = load_yaml(path) + updated, added = add_source_to_resource(resource, source, dedupe_keys=dedupe_keys) + if added: + dump_yaml(path, updated) + return added diff --git a/tests/test_coverage.py b/tests/test_coverage.py new file mode 100644 index 00000000..8a12bc4e --- /dev/null +++ b/tests/test_coverage.py @@ -0,0 +1,123 @@ +"""Unit tests for omi.creation.coverage (store coverage reporting).""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import yaml + +from omi.creation.coverage import ( + coverage_report, + index_store_resources, + resource_completeness, +) + +if TYPE_CHECKING: + from pathlib import Path + + +# ---------- resource_completeness ---------- + + +def _complete_resource() -> dict: + """Return a fully-documented resource dict for use as a test baseline.""" + return { + "name": "grid.t", + "title": "Grid table", + "description": "A real description.", + "schema": {"fields": [{"name": "id", "description": "primary id"}]}, + } + + +def test_completeness_complete_when_all_filled() -> None: + """A fully-filled resource is classified complete with no reasons.""" + state, reasons = resource_completeness(_complete_resource()) + assert state == "complete" + assert reasons == [] + + +def test_completeness_skeleton_on_blank_required_field() -> None: + """A blank required field makes the resource a skeleton.""" + res = _complete_resource() + res["description"] = "" + state, reasons = resource_completeness(res) + assert state == "skeleton" + assert any("description" in r for r in reasons) + + +def test_completeness_skeleton_on_placeholder() -> None: + """A TODO placeholder in a required field makes the resource a skeleton.""" + res = _complete_resource() + res["title"] = "TODO: Add title" + state, _ = resource_completeness(res) + assert state == "skeleton" + + +def test_completeness_skeleton_on_missing_field_description() -> None: + """A schema field without a description makes the resource a skeleton.""" + res = _complete_resource() + res["schema"]["fields"][0]["description"] = "" + state, _ = resource_completeness(res) + assert state == "skeleton" + + +def test_completeness_can_ignore_field_descriptions() -> None: + """Field-description checks can be disabled via the flag.""" + res = _complete_resource() + res["schema"]["fields"][0]["description"] = "TODO: Add description" + state, _ = resource_completeness(res, require_field_descriptions=False) + assert state == "complete" + + +# ---------- store indexing + coverage_report ---------- + + +def _write_resource(base: Path, dataset_id: str, resource: dict) -> None: + """Write a resource YAML plus a matching dataset stub under base.""" + safe = str(resource["name"]).replace(".", "_") + d = base / "resources" / dataset_id + d.mkdir(parents=True, exist_ok=True) + (d / f"{safe}.resource.yaml").write_text(yaml.safe_dump(resource), encoding="utf-8") + ds = base / "datasets" + ds.mkdir(parents=True, exist_ok=True) + (ds / f"{dataset_id}.dataset.yaml").write_text( + yaml.safe_dump({"version": "OEMetadata-2.0.4", "dataset": {"name": dataset_id}}), + encoding="utf-8", + ) + + +def test_index_store_resources_maps_names(tmp_path: Path) -> None: + """The store index maps each resource name to its YAML path.""" + _write_resource(tmp_path, "egon_grid", _complete_resource()) + index = index_store_resources(tmp_path) + assert "grid.t" in index + assert index["grid.t"].exists() + + +def test_coverage_report_classifies_missing_skeleton_complete_orphan(tmp_path: Path) -> None: + """coverage_report sorts resources into complete/skeleton/missing/orphan.""" + _write_resource(tmp_path, "egon_grid", _complete_resource()) + skel = _complete_resource() + skel["name"] = "grid.skel" + skel["description"] = "" + _write_resource(tmp_path, "egon_grid", skel) + orphan = _complete_resource() + orphan["name"] = "grid.orphan" + _write_resource(tmp_path, "egon_grid", orphan) + + expected = ["grid.t", "grid.skel", "grid.absent"] + report = coverage_report(tmp_path, expected) + + assert report.complete == ["grid.t"] + assert [s.name for s in report.skeleton] == ["grid.skel"] + assert report.missing == ["grid.absent"] + assert "grid.orphan" in report.orphans + assert report.ok is False + + +def test_coverage_report_ok_when_nothing_missing(tmp_path: Path) -> None: + """coverage_report.ok is True when every expected resource exists.""" + _write_resource(tmp_path, "egon_grid", _complete_resource()) + report = coverage_report(tmp_path, ["grid.t"]) + assert report.ok is True + assert report.summary().startswith("complete=1") diff --git a/tests/test_merge_update.py b/tests/test_merge_update.py new file mode 100644 index 00000000..33da4fe1 --- /dev/null +++ b/tests/test_merge_update.py @@ -0,0 +1,57 @@ +"""Tests for the non-destructive DB merge-update wrapper in omi.creation.init.""" + +from __future__ import annotations + +from omi.creation.init import update_resource_from_db_skeleton + + +def _resource() -> dict: + """Return a resource with a human description and a soon-to-be-dropped column.""" + return { + "name": "grid.t", + "title": "Grid table", + "description": "Human-written description.", + "schema": { + "primaryKey": ["id"], + "fields": [ + {"name": "id", "description": "primary id", "type": "integer", "unit": "none"}, + {"name": "gone", "description": "was here", "type": "string", "unit": "none"}, + ], + }, + } + + +def _db_skeleton() -> dict: + """Return a DB skeleton with 'id' unchanged, a new 'extra', and 'gone' dropped.""" + return { + "name": "grid.t", + "schema": { + "primaryKey": ["id"], + "foreignKeys": [], + "fields": [ + {"name": "id", "description": "TODO: Add description", "type": "integer", "unit": "none"}, + {"name": "extra", "description": "TODO: Add description", "type": "string", "unit": "none"}, + ], + }, + } + + +def test_merge_preserves_human_description_and_adds_new_column() -> None: + """Human descriptions survive; new DB columns arrive with a TODO placeholder.""" + updated, report = update_resource_from_db_skeleton(_resource(), _db_skeleton()) + fields = {f["name"]: f for f in updated["schema"]["fields"]} + + assert fields["id"]["description"] == "primary id" + assert "extra" in fields + assert "TODO" in fields["extra"]["description"] + assert "gone" in fields + assert report["missing_in_yaml"] == ["extra"] + assert report["missing_in_db"] == ["gone"] + + +def test_merge_does_not_mutate_input() -> None: + """The input resource is not mutated by the merge.""" + original = _resource() + update_resource_from_db_skeleton(original, _db_skeleton()) + names = [f["name"] for f in original["schema"]["fields"]] + assert names == ["id", "gone"] diff --git a/tests/test_sources.py b/tests/test_sources.py new file mode 100644 index 00000000..6cc7d35f --- /dev/null +++ b/tests/test_sources.py @@ -0,0 +1,97 @@ +"""Unit tests for omi.creation.sources (provenance source helpers).""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import yaml + +from omi.creation.sources import ( + add_source_to_resource, + add_source_to_resource_file, + build_external_source, + build_internal_source, +) + +if TYPE_CHECKING: + from pathlib import Path + + +# ---------- constructors ---------- + + +def test_build_external_source_has_spec_keys_and_title() -> None: + """An external source keeps title/description/path and stub spec keys.""" + src = build_external_source("MaStR", description="Registry", path="https://mastr.de") + assert src["title"] == "MaStR" + assert src["description"] == "Registry" + assert src["path"] == "https://mastr.de" + assert src["authors"] == [] + assert src["publicationYear"] is None + assert src["sourceLicenses"] == [] + + +def test_build_internal_source_references_table() -> None: + """An internal source references the table and uses only spec keys.""" + src = build_internal_source("grid.egon_etrago_bus") + assert "grid.egon_etrago_bus" in src["title"] + assert "grid.egon_etrago_bus" in src["description"] + assert set(src) == {"title", "description", "path", "authors", "publicationYear", "sourceLicenses"} + + +# ---------- add_source_to_resource ---------- + + +def test_add_source_appends_and_does_not_mutate_input() -> None: + """Adding a source returns a copy and leaves the input untouched.""" + resource = {"name": "grid.t", "sources": []} + src = build_external_source("OSM", path="https://osm.org") + updated, added = add_source_to_resource(resource, src) + assert added is True + assert len(updated["sources"]) == 1 + assert resource["sources"] == [] + + +def test_add_source_creates_sources_list_when_absent() -> None: + """A missing 'sources' key is created on first add.""" + resource = {"name": "grid.t"} + updated, added = add_source_to_resource(resource, build_external_source("X")) + assert added is True + assert isinstance(updated["sources"], list) + assert updated["sources"][0]["title"] == "X" + + +def test_add_source_dedupes_on_title_and_path() -> None: + """An equivalent source (same title+path) is not added twice.""" + src = build_external_source("MaStR", path="https://mastr.de") + resource = {"name": "grid.t", "sources": [src]} + updated, added = add_source_to_resource(resource, build_external_source("MaStR", path="https://mastr.de")) + assert added is False + assert len(updated["sources"]) == 1 + + +def test_add_source_same_title_different_path_is_new() -> None: + """Same title but different path counts as a distinct source.""" + resource = {"name": "grid.t", "sources": [build_external_source("MaStR", path="a")]} + updated, added = add_source_to_resource(resource, build_external_source("MaStR", path="b")) + assert added is True + assert len(updated["sources"]) == 2 + + +# ---------- add_source_to_resource_file ---------- + + +def test_add_source_to_file_writes_and_is_idempotent(tmp_path: Path) -> None: + """Writing a source to a file persists once and is a no-op on repeat.""" + res_path = tmp_path / "grid_t.resource.yaml" + res_path.write_text(yaml.safe_dump({"name": "grid.t", "sources": []}), encoding="utf-8") + + src = build_internal_source("grid.egon_etrago_bus") + assert add_source_to_resource_file(res_path, src) is True + + reloaded = yaml.safe_load(res_path.read_text(encoding="utf-8")) + assert len(reloaded["sources"]) == 1 + + assert add_source_to_resource_file(res_path, src) is False + reloaded = yaml.safe_load(res_path.read_text(encoding="utf-8")) + assert len(reloaded["sources"]) == 1