Relax value-presence constraint per model (Observation/Measurement/Metadata)

omop_alchemy/cdm/base/column_mixins.py

@@ -46,49 +46,22 @@ class ValueMixin:
     """
     `ValueMixin`
 
-    Encodes the OMOP pattern where a value may be represented *either numerically or categorically*.
+    Encodes the OMOP pattern where a value may be represented *either numerically or categorically*,
+    via `value_as_number` and/or `value_as_concept_id`.
 
-    Structural guarantees:
-
-    - at least one value must be present
-    - enforced via a check constraint
-    - validated at assignment time
+    This mixin only supplies the two columns. It does NOT enforce that either is
+    populated: OMOP CDM v5.4 does not mandate a value on every row for every
+    table this mixin is used by (e.g. `Measurement`, `Metadata`), and some
+    tables represent a third value form (`Observation.value_as_string`) that
+    this mixin has no way to know about. Subclasses that need a "some value
+    must be present" guarantee should declare their own check constraint and
+    `@validates` covering exactly the value columns they actually have.
 
     This helps when building generic tooling that needs to handle values flexibly but then normalise for analysis.
-
-    Examples
-    --------
-    >>> from omop_alchemy.cdm.model.clinical.measurement import Measurement
-    >>> m = Measurement()
-    >>> m.value_as_number = 42.0
-    >>> m.value_as_concept_id = None  # OK
-    >>> m.value_as_number = None  # Raises ValueError
-
     """
     value_as_number: so.Mapped[Optional[float]] = so.mapped_column(sa.Float, nullable=True)
     value_as_concept_id: so.Mapped[Optional[int]] = so.mapped_column(sa.ForeignKey("concept.concept_id"), nullable=True)
 
-    __table_args__ = (
-        sa.CheckConstraint(
-            "value_as_number IS NOT NULL OR value_as_concept_id IS NOT NULL",
-            name="ck_value_present",
-        ),
-    )
-
-    @so.validates("value_as_number", "value_as_concept_id")
-    def _validate_value(self, key, value):
-        other = (
-            self.value_as_concept_id
-            if key == "value_as_number"
-            else self.value_as_number
-        )
-
-        if value is None and other is None:
-            raise ValueError(
-                "At least one of value_as_number or value_as_concept_id must be set"
-            )
-        return value
-
 class DatedEvent:
     """
     `DatedEvent`

omop_alchemy/cdm/model/clinical/measurement.py

@@ -18,7 +18,6 @@
 class Measurement(Base, CDMTableBase, ValueMixin):
     __tablename__ = "measurement"
     __table_args__ = merge_table_args(
-        ValueMixin.__table_args__,
         omop_index(__tablename__, "person_id", cluster=True),
         omop_index(__tablename__, "measurement_concept_id"),
         omop_index(__tablename__, "visit_occurrence_id"),

omop_alchemy/cdm/model/clinical/observation.py

@@ -18,7 +18,10 @@
 class Observation(Base, CDMTableBase, ValueMixin):
     __tablename__ = "observation"
     __table_args__ = merge_table_args(
-        ValueMixin.__table_args__,
+        sa.CheckConstraint(
+            "value_as_number IS NOT NULL OR value_as_concept_id IS NOT NULL OR value_as_string IS NOT NULL",
+            name="ck_value_present",
+        ),
         omop_index(__tablename__, "person_id", cluster=True),
         omop_index(__tablename__, "observation_concept_id"),
         omop_index(__tablename__, "visit_occurrence_id"),
@@ -54,3 +57,17 @@ def modifier_of_event_id(self) -> Optional[int]:
     @hybrid_property
     def modifier_of_field_concept_id(self) -> Optional[int]:
         return self.obs_event_field_concept_id
+
+    @so.validates("value_as_number", "value_as_concept_id", "value_as_string")
+    def _validate_value(self, key, value):
+        others = [
+            getattr(self, other_key)
+            for other_key in ("value_as_number", "value_as_concept_id", "value_as_string")
+            if other_key != key
+        ]
+        if value is None and all(other is None for other in others):
+            raise ValueError(
+                "At least one of value_as_number, value_as_concept_id, or "
+                "value_as_string must be set"
+            )
+        return value

omop_alchemy/cdm/model/metadata/metadata.py

@@ -17,7 +17,6 @@
 class Metadata(CDMTableBase, Base, ValueMixin):
     __tablename__ = "metadata"
     __table_args__ = merge_table_args(
-        ValueMixin.__table_args__,
         omop_index(__tablename__, "metadata_concept_id", cluster=True),
         omop_index(__tablename__, "metadata_type_concept_id"),
     )

tests/test_value_mixin.py

@@ -0,0 +1,142 @@
+"""Tests for the relaxed ValueMixin: per-model value requirements.
+
+Observation requires one of value_as_number/value_as_concept_id/value_as_string.
+Measurement and Metadata require no value column to be populated at all.
+"""
+
+from datetime import date
+
+import pytest
+import sqlalchemy.exc as sa_exc
+
+from omop_alchemy.cdm.model.clinical.measurement import Measurement
+from omop_alchemy.cdm.model.clinical.observation import Observation
+from omop_alchemy.cdm.model.metadata.metadata import Metadata
+
+from .conftest import _concept_id
+
+
+@pytest.fixture
+def type_concept_id(session):
+    return _concept_id(session, domain_id="Type Concept")
+
+
+@pytest.fixture
+def some_concept_id(session):
+    return _concept_id(session, domain_id="Condition")
+
+
+def _make_observation(*, observation_id, type_concept_id, some_concept_id, **value_kwargs):
+    return Observation(
+        observation_id=observation_id,
+        person_id=1,
+        observation_concept_id=some_concept_id,
+        observation_date=date(2020, 6, 1),
+        observation_type_concept_id=type_concept_id,
+        **value_kwargs,
+    )
+
+
+def _make_measurement(*, measurement_id, type_concept_id, some_concept_id, **value_kwargs):
+    return Measurement(
+        measurement_id=measurement_id,
+        person_id=1,
+        measurement_concept_id=some_concept_id,
+        measurement_date=date(2020, 6, 1),
+        measurement_type_concept_id=type_concept_id,
+        **value_kwargs,
+    )
+
+
+class TestObservationValueRequirement:
+    def test_value_as_string_alone_is_sufficient(self, session, type_concept_id, some_concept_id):
+        """OMOP allows Observation's value via value_as_string alone."""
+        obs = _make_observation(
+            observation_id=9001,
+            type_concept_id=type_concept_id,
+            some_concept_id=some_concept_id,
+            value_as_string="elevated",
+        )
+        session.add(obs)
+        session.commit()
+        assert obs.observation_id == 9001
+
+    def test_value_as_number_alone_is_sufficient(self, session, type_concept_id, some_concept_id):
+        obs = _make_observation(
+            observation_id=9002,
+            type_concept_id=type_concept_id,
+            some_concept_id=some_concept_id,
+            value_as_number=42.0,
+        )
+        session.add(obs)
+        session.commit()
+        assert obs.observation_id == 9002
+
+    def test_value_as_concept_id_alone_is_sufficient(
+        self, session, type_concept_id, some_concept_id
+    ):
+        obs = _make_observation(
+            observation_id=9003,
+            type_concept_id=type_concept_id,
+            some_concept_id=some_concept_id,
+            value_as_concept_id=some_concept_id,
+        )
+        session.add(obs)
+        session.commit()
+        assert obs.observation_id == 9003
+
+    def test_assigning_all_three_none_raises(self, session, type_concept_id, some_concept_id):
+        """Assignment-time validator still rejects an all-NULL value on Observation."""
+        obs = _make_observation(
+            observation_id=9004, type_concept_id=type_concept_id, some_concept_id=some_concept_id
+        )
+        with pytest.raises(ValueError, match="At least one of"):
+            obs.value_as_number = None
+
+    def test_no_value_at_all_fails_at_flush(self, session, type_concept_id, some_concept_id):
+        """Never assigning any value column also fails, at DB flush time."""
+        obs = _make_observation(
+            observation_id=9005, type_concept_id=type_concept_id, some_concept_id=some_concept_id
+        )
+        session.add(obs)
+        with pytest.raises(sa_exc.IntegrityError):
+            session.commit()
+
+
+class TestMeasurementNoValueRequirement:
+    def test_no_value_columns_set_succeeds(self, session, type_concept_id, some_concept_id):
+        """Measurement has no CDM-mandated value; a bare result row is valid."""
+        meas = _make_measurement(
+            measurement_id=9001, type_concept_id=type_concept_id, some_concept_id=some_concept_id
+        )
+        session.add(meas)
+        session.commit()
+        assert meas.measurement_id == 9001
+        assert meas.value_as_number is None
+        assert meas.value_as_concept_id is None
+
+    def test_value_as_number_still_settable(self, session, type_concept_id, some_concept_id):
+        meas = _make_measurement(
+            measurement_id=9002,
+            type_concept_id=type_concept_id,
+            some_concept_id=some_concept_id,
+            value_as_number=98.6,
+        )
+        session.add(meas)
+        session.commit()
+        assert meas.value_as_number == 98.6
+
+
+class TestMetadataNoValueRequirement:
+    def test_no_value_columns_set_succeeds(self, session):
+        concept_id = _concept_id(session, domain_id="Metadata")
+        meta = Metadata(
+            metadata_id=9001,
+            metadata_concept_id=concept_id,
+            metadata_type_concept_id=concept_id,
+            name="fixture-metadata",
+        )
+        session.add(meta)
+        session.commit()
+        assert meta.metadata_id == 9001
+        assert meta.value_as_string is None