Repository navigation
Move RelationshipClass/RelationshipMapping onto the CDM auxiliary schema - #45
Open
nicoloesch wants to merge 7 commits into
Open
nicoloesch wants to merge 7 commits into
nicoloesch wants to merge 7 commits into
Conversation
Both tables now inherit CDMExtensionTableBase instead of CDMTableBase, and their predicate_kind enum lives in EXTENSION_SCHEMA instead of Role.PRIMARY. RelationshipMapping's FK to relationship_class becomes a plain qualified string (there's no Role.EXTENSION; role_fk() is deliberately Role-only). relationship_classification() claims and ensures the extension schema itself when it resolves its own engine; a caller injecting its own engine/connection is responsible for including that claim, same convention as the existing staging_schema_claim(). Tests building their own scoped engine via scoped_test_schema()/ create_engines() now also claim a fresh, test-isolated extension schema alongside their existing staging claim, since schema_claims aren't inherited across separate create_engine() calls on the same resolved.
Verified against a real Postgres connection: Base.metadata.create_all()
creates every custom dialect-level type registered anywhere on the shared
Base.metadata (e.g. RelationshipClass's predicate_kind enum), regardless
of its own tables= filter. Once RelationshipClass/RelationshipMapping
moved to the extension schema, every test building its own engine via
scoped_test_schema()/create_engines()/create_engine() needed that schema
tag claimed and physically created too, not just the tables it actually
asked for.
Adds a shared extension_schema_claim() test helper (fixtures/helpers.py):
a fresh, test-isolated physical schema per test, claimed under a fixed
owner ("omop_graph") so sequential test files claiming the same tag on
the same connection don't collide on ownership, and with
register_claims=False so none of them persist a provenance row that
could drift against a sibling test's own differently-named schema.
test_schema_provenance_guard.py is the one exception: it deliberately
exercises relationship_classification()'s own internal claim (the
engine=None path in cli.py), so it claims the same literal schema name
and owner that path auto-derives, with real registration enabled.
main's own cli.py refactor (independent of this branch) replaced relationship_classification()'s engine= injection with a resolved-only interface that always builds its engine via open_cdm_sessions() -> create_cdm_engines(), which had no way to accept extra schema_claims. That meant the extension tag always resolved to the literal CDM_EXTENSION_SCHEMA default, with no way to point it at a test-isolated physical schema. relationship_classification()/open_cdm_sessions()/ _create_cdm_sessions() now accept and forward a schema_claims parameter, paired with the same addition on omop-alchemy's create_cdm_engines(). Every affected test now passes its own ext_claim through, and resets the shared CDM_EXTENSION_SCHEMA registry row around it (register_claims=True is the default, needed so relationship_classification() can actually create the schema it writes into) so one test's physical schema doesn't leak into whichever test claims the tag next. Also: the plain SQLite mock engine in fixtures/mock_cdm.py needed a cdm_extension entry in its hand-built schema_translate_map now that RelationshipClass/RelationshipMapping live there instead of Role.PRIMARY.
nicoloesch
changed the title
Pulls in DuckDB classification support (#51). Resolved the conflict in extensions/omop_alchemy.py by keeping this branch's CDMExtensionTableBase inheritance while adopting main's DuckDB-safe _predicate_kind_enum(), parameterized on schema instead of hardcoding Role.PRIMARY. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Avoids colliding with OHDSI's own Extension/Expansion mechanism (working-group-governed table sets like Oncology/Imaging/GIS); these are package-owned add-ons, not OHDSI-recognized extensions. Also threads physical_schemas through relationship_classification()'s DuckDB-affected calls: the auxiliary schema is a caller-claimed tag, not one of resolved's own configured Role tags, so a dialect that cannot cross schemas needs its physical name explicitly to classify the FK to vocab as feasible or deferred. Fixes the same gap in an ad hoc SQLite fixture's hardcoded schema_translate_map. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pyproject.tomlversion update foromop-alchemySummary
Fixes #42
RelationshipClass/RelationshipMappinghad no schema tag of their own, so omop-alchemy's maintenance tooling mis-routed them in any process importing both packages. They're now proper auxiliary tables (package-owned add-ons, not OHDSI's own CDM Extension mechanism).CDMAuxiliaryTableBaseinstead of the old CDM-table decorator.db/session.pybuilds its own engines directly: staging + auxiliary claim only, nocreate_cdm_engines()/MAINTENANCE_SCHEMA.Checklist
fix)uv run pytest -q)uv run pytest -q -m postgresql)uv run ruff check .)