diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6bd22..3a1ec00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## v0.3.0 Bugfixes +- Incorrect references for payer_plan_period_id fields in CDM 5.4 ([#12](https://github.com/thehyve/omop-cdm/issues/12)) - The stem table field `start_datetime` is now nullable in CDM 5.3 and 5.4 ([#14](https://github.com/thehyve/omop-cdm/issues/14)) ## v0.2.0 diff --git a/src/omop_cdm/dynamic/cdm54/health_economics.py b/src/omop_cdm/dynamic/cdm54/health_economics.py index b54b970..295f936 100644 --- a/src/omop_cdm/dynamic/cdm54/health_economics.py +++ b/src/omop_cdm/dynamic/cdm54/health_economics.py @@ -15,7 +15,7 @@ class BasePayerPlanPeriodCdm54: __tablename__ = "payer_plan_period" __table_args__ = {"schema": CDM_SCHEMA} - payer_plan_period_id: Mapped[int] = mapped_column(ForeignKey(FK_PERSON_ID), primary_key=True, sort_order=100) + payer_plan_period_id: Mapped[int] = mapped_column(Integer, primary_key=True, sort_order=100) person_id: Mapped[int] = mapped_column(ForeignKey(FK_PERSON_ID), index=True, sort_order=200) payer_plan_period_start_date: Mapped[datetime.date] = mapped_column(Date, sort_order=300) payer_plan_period_end_date: Mapped[datetime.date] = mapped_column(Date, sort_order=400) @@ -37,10 +37,6 @@ class BasePayerPlanPeriodCdm54: def payer_concept(cls) -> Mapped["Concept"]: return relationship("Concept", foreign_keys="PayerPlanPeriod.payer_concept_id") - @declared_attr - def payer_plan_period(cls) -> Mapped["Person"]: - return relationship("Person", foreign_keys="PayerPlanPeriod.payer_plan_period_id") - @declared_attr def payer_source_concept(cls) -> Mapped["Concept"]: return relationship("Concept", foreign_keys="PayerPlanPeriod.payer_source_concept_id") @@ -94,7 +90,9 @@ class BaseCostCdm54: paid_by_primary: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric, sort_order=1400) paid_ingredient_cost: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric, sort_order=1500) paid_dispensing_fee: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric, sort_order=1600) - payer_plan_period_id: Mapped[Optional[int]] = mapped_column(Integer, sort_order=1700) + payer_plan_period_id: Mapped[Optional[int]] = mapped_column( + ForeignKey(f"{CDM_SCHEMA}.payer_plan_period.payer_plan_period_id"), sort_order=1700 + ) amount_allowed: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric, sort_order=1800) revenue_code_concept_id: Mapped[Optional[int]] = mapped_column(ForeignKey(FK_CONCEPT_ID), sort_order=1900) revenue_code_source_value: Mapped[Optional[str]] = mapped_column(String(50), sort_order=2000) diff --git a/src/omop_cdm/regular/cdm54/tables.py b/src/omop_cdm/regular/cdm54/tables.py index 8cc409c..5cfe3a3 100644 --- a/src/omop_cdm/regular/cdm54/tables.py +++ b/src/omop_cdm/regular/cdm54/tables.py @@ -850,7 +850,7 @@ class PayerPlanPeriod(Base): __tablename__ = "payer_plan_period" __table_args__ = {"schema": CDM_SCHEMA} - payer_plan_period_id: Mapped[int] = mapped_column(ForeignKey(FK_PERSON_ID), primary_key=True) + payer_plan_period_id: Mapped[int] = mapped_column(Integer, primary_key=True) person_id: Mapped[int] = mapped_column(ForeignKey(FK_PERSON_ID), index=True) payer_plan_period_start_date: Mapped[datetime.date] = mapped_column(Date) payer_plan_period_end_date: Mapped[datetime.date] = mapped_column(Date) @@ -869,7 +869,6 @@ class PayerPlanPeriod(Base): stop_reason_source_concept_id: Mapped[Optional[int]] = mapped_column(ForeignKey(FK_CONCEPT_ID)) payer_concept: Mapped["Concept"] = relationship("Concept", foreign_keys="PayerPlanPeriod.payer_concept_id") - payer_plan_period: Mapped["Person"] = relationship("Person", foreign_keys="PayerPlanPeriod.payer_plan_period_id") payer_source_concept: Mapped["Concept"] = relationship( "Concept", foreign_keys="PayerPlanPeriod.payer_source_concept_id" ) @@ -910,7 +909,9 @@ class Cost(Base): paid_by_primary: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric) paid_ingredient_cost: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric) paid_dispensing_fee: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric) - payer_plan_period_id: Mapped[Optional[int]] = mapped_column(Integer) + payer_plan_period_id: Mapped[Optional[int]] = mapped_column( + ForeignKey(f"{CDM_SCHEMA}.payer_plan_period.payer_plan_period_id") + ) amount_allowed: Mapped[Optional[decimal.Decimal]] = mapped_column(Numeric) revenue_code_concept_id: Mapped[Optional[int]] = mapped_column(ForeignKey(FK_CONCEPT_ID)) revenue_code_source_value: Mapped[Optional[str]] = mapped_column(String(50)) diff --git a/tests/conftest.py b/tests/conftest.py index d2f3993..2353e72 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,9 @@ from contextlib import contextmanager -from typing import Set +from typing import Callable, Set import pytest -from sqlalchemy import Connection, Engine, MetaData, create_engine +from sqlalchemy import Connection, Engine, MetaData, create_engine, select +from sqlalchemy.orm import Session from sqlalchemy.sql.ddl import CreateSchema, DropSchema from testcontainers.postgres import PostgresContainer @@ -38,3 +39,16 @@ def temp_schemas(engine: Engine, schemas: Set[str]): def create_all_tables(engine: Engine, metadata: MetaData) -> None: with engine.begin() as conn: metadata.create_all(bind=conn) + + +def validate_relationships(engine: Engine, table: Callable) -> None: + """ + Query any table to make SQLAlchemy validate table relationships. + + While SQLAlchemy's Metadata.create_all method will create the tables + in the database, it can still be that ORM FK relationships are + configured incorrectly. This will only become apparent upon first + using an ORM query. Hence, this simple session interaction. + """ + with Session(engine) as session: + session.execute(select(table)) diff --git a/tests/omop_cdm/dynamic/cdm531/test_cdm531.py b/tests/omop_cdm/dynamic/cdm531/test_cdm531.py index 504fa18..8837e6e 100644 --- a/tests/omop_cdm/dynamic/cdm531/test_cdm531.py +++ b/tests/omop_cdm/dynamic/cdm531/test_cdm531.py @@ -2,7 +2,7 @@ from sqlalchemy import Engine, inspect from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.dynamic.cdm_definitions import cdm531 from tests.omop_cdm.table_sets import CDM531_NON_VOCAB, CUSTOM, VOCAB @@ -21,3 +21,4 @@ def test_create_tables_cdm531(cdm531_engine: Engine): cdm_tables = inspect(cdm531_engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM531_NON_VOCAB | CUSTOM + validate_relationships(cdm531_engine, cdm531.Person) diff --git a/tests/omop_cdm/dynamic/cdm54/test_cdm54.py b/tests/omop_cdm/dynamic/cdm54/test_cdm54.py index 8261960..d8f2d21 100644 --- a/tests/omop_cdm/dynamic/cdm54/test_cdm54.py +++ b/tests/omop_cdm/dynamic/cdm54/test_cdm54.py @@ -2,7 +2,7 @@ from sqlalchemy import Engine, inspect from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.dynamic.cdm_definitions import cdm54 from tests.omop_cdm.table_sets import CDM54_NON_VOCAB, CUSTOM, VOCAB @@ -21,3 +21,4 @@ def test_create_tables_cdm54(cdm54_engine: Engine): cdm_tables = inspect(cdm54_engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM54_NON_VOCAB | CUSTOM + validate_relationships(cdm54_engine, cdm54.Person) diff --git a/tests/omop_cdm/dynamic/cdm54_custom/test_cdm54_custom.py b/tests/omop_cdm/dynamic/cdm54_custom/test_cdm54_custom.py index 7e6795c..43123e8 100644 --- a/tests/omop_cdm/dynamic/cdm54_custom/test_cdm54_custom.py +++ b/tests/omop_cdm/dynamic/cdm54_custom/test_cdm54_custom.py @@ -2,7 +2,7 @@ from sqlalchemy import BIGINT, Engine, inspect from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.dynamic.cdm_definitions import cdm54_custom from tests.omop_cdm.table_sets import CDM54_NON_VOCAB, CUSTOM @@ -27,6 +27,7 @@ def _create_custom_cdm_tables(cdm54_custom_engine: Engine): def test_custom_table_is_created(cdm54_custom_engine: Engine): cdm_tables = inspect(cdm54_custom_engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(cdm_tables) == CDM54_NON_VOCAB | CUSTOM | {"cloudspine"} + validate_relationships(cdm54_custom_engine, cdm54_custom.Person) @pytest.mark.usefixtures("_create_custom_cdm_tables") diff --git a/tests/omop_cdm/dynamic/cdm54_plus_legacy/test_legacy.py b/tests/omop_cdm/dynamic/cdm54_plus_legacy/test_legacy.py index 467fbbe..f543b19 100644 --- a/tests/omop_cdm/dynamic/cdm54_plus_legacy/test_legacy.py +++ b/tests/omop_cdm/dynamic/cdm54_plus_legacy/test_legacy.py @@ -2,7 +2,7 @@ from sqlalchemy import Engine, inspect from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.dynamic.cdm_definitions import cdm54_plus_legacy from tests.omop_cdm.table_sets import CDM54_NON_VOCAB, CUSTOM, LEGACY, VOCAB @@ -23,3 +23,4 @@ def test_create_tables_cdm54_plus_legacy(cdm54_legacy_engine: Engine): SCHEMA_MAP[CDM_SCHEMA] ) assert set(cdm_tables) == VOCAB | CDM54_NON_VOCAB | LEGACY | CUSTOM + validate_relationships(cdm54_legacy_engine, cdm54_plus_legacy.Person) diff --git a/tests/omop_cdm/dynamic/cdm600/test_cdm600.py b/tests/omop_cdm/dynamic/cdm600/test_cdm600.py index 83135b9..b07e8c8 100644 --- a/tests/omop_cdm/dynamic/cdm600/test_cdm600.py +++ b/tests/omop_cdm/dynamic/cdm600/test_cdm600.py @@ -2,7 +2,7 @@ from sqlalchemy import Engine, inspect from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.dynamic.cdm_definitions import cdm600 from tests.omop_cdm.table_sets import CDM600_NON_VOCAB, CUSTOM, VOCAB @@ -21,3 +21,4 @@ def test_create_tables_cdm600(cdm600_engine: Engine): cdm_tables = inspect(cdm600_engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM600_NON_VOCAB | CUSTOM + validate_relationships(cdm600_engine, cdm600.Person) diff --git a/tests/omop_cdm/regular/cdm531/test_cdm531.py b/tests/omop_cdm/regular/cdm531/test_cdm531.py index 9411729..fc50597 100644 --- a/tests/omop_cdm/regular/cdm531/test_cdm531.py +++ b/tests/omop_cdm/regular/cdm531/test_cdm531.py @@ -3,7 +3,7 @@ import src.omop_cdm.regular.cdm531 as cdm531 from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.table_sets import CDM531_NON_VOCAB, VOCAB SCHEMA_MAP = {VOCAB_SCHEMA: "vocab531", CDM_SCHEMA: "cdm531"} @@ -22,3 +22,4 @@ def test_create_tables_cdm531_regular(cdm531_regular_engine: Engine): cdm_tables = inspect(engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM531_NON_VOCAB + validate_relationships(cdm531_regular_engine, cdm531.Person) diff --git a/tests/omop_cdm/regular/cdm54/test_cdm54.py b/tests/omop_cdm/regular/cdm54/test_cdm54.py index 5e8824d..5d7013c 100644 --- a/tests/omop_cdm/regular/cdm54/test_cdm54.py +++ b/tests/omop_cdm/regular/cdm54/test_cdm54.py @@ -3,7 +3,7 @@ import src.omop_cdm.regular.cdm54 as cdm54 from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.table_sets import CDM54_NON_VOCAB, VOCAB SCHEMA_MAP = {VOCAB_SCHEMA: "vocab54", CDM_SCHEMA: "cdm54"} @@ -22,3 +22,4 @@ def test_create_tables_cdm54_regular(cdm54_regular_engine: Engine): cdm_tables = inspect(engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM54_NON_VOCAB + validate_relationships(cdm54_regular_engine, cdm54.Person) diff --git a/tests/omop_cdm/regular/cdm600/test_cdm600.py b/tests/omop_cdm/regular/cdm600/test_cdm600.py index 8ba5e58..76e2e39 100644 --- a/tests/omop_cdm/regular/cdm600/test_cdm600.py +++ b/tests/omop_cdm/regular/cdm600/test_cdm600.py @@ -3,7 +3,7 @@ import src.omop_cdm.regular.cdm600 as cdm600 from src.omop_cdm.constants import CDM_SCHEMA, VOCAB_SCHEMA -from tests.conftest import create_all_tables, temp_schemas +from tests.conftest import create_all_tables, temp_schemas, validate_relationships from tests.omop_cdm.table_sets import CDM600_NON_VOCAB, VOCAB SCHEMA_MAP = {VOCAB_SCHEMA: "vocab600", CDM_SCHEMA: "cdm600"} @@ -22,3 +22,4 @@ def test_create_tables_cdm600_regular(cdm600_regular_engine: Engine): cdm_tables = inspect(engine).get_table_names(SCHEMA_MAP[CDM_SCHEMA]) assert set(vocab_tables) == VOCAB assert set(cdm_tables) == CDM600_NON_VOCAB + validate_relationships(cdm600_regular_engine, cdm600.Person)