Skip to content

Measurement_Event.to_dict/to_json violate SerialisableTableInterface's contract (LSP) #41

Description

@nicoloesch

Pre-flight

  • I searched existing issues and this is not a duplicate.

Summary

Measurement_Event(ClinicalEvent, Measurement)inomop_alchemy/cdm/handlers/timeline/event_timeline.pyinheritsto_dict/to_jsonfromClinicalEvent, whose signatures don't accept the keyword arguments that SerialisableTableInterface(fromorm-loader, a base of Measurement) declares. Any code holding a SerialisableTableInterface-typed reference to a Measurement_Eventand calling.to_dict(include_nulls=True)(or anyto_jsonkwarg) will raiseTypeError` at runtime, breaking substitutability.

Caught by ty check as of ty>=0.0.62:

error[invalid-method-override]: Base classes for class `Measurement_Event` define method `to_dict` incompatibly
   --> omop_alchemy/cdm/handlers/timeline/event_timeline.py:204:7
    |
204 | class Measurement_Event(ClinicalEvent, Measurement):
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ClinicalEvent.to_dict` is incompatible with `SerialisableTableInterface.to_dict`
    |
   ::: omop_alchemy/cdm/handlers/timeline/event_timeline.py:173:9
    |
173 |     def to_dict(self: ClinicalEventProtocol) -> dict[str, Any]:
    |         ------- `ClinicalEvent.to_dict` defined here
    |
   ::: orm_loader/tables/serialisable_table.py:56:9
    |
 56 |     def to_dict(
    |         ------- `SerialisableTableInterface.to_dict` defined here
    |
info: This violates the Liskov Substitution Principle

error[invalid-method-override]: Base classes for class `Measurement_Event` define method `to_json` incompatibly
   --> omop_alchemy/cdm/handlers/timeline/event_timeline.py:204:7
    |
204 | class Measurement_Event(ClinicalEvent, Measurement):
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ClinicalEvent.to_json` is incompatible with `SerialisableTableInterface.to_json`
    |
   ::: omop_alchemy/cdm/handlers/timeline/event_timeline.py:189:9
    |
189 |     def to_json(self: ClinicalEventProtocol) -> str:
    |         ------- `ClinicalEvent.to_json` defined here
    |
   ::: orm_loader/tables/serialisable_table.py:98:9
    |
 98 |     def to_json(self, **kwargs: Unpack[ToDictKwargs]) -> str:
    |         ------- `SerialisableTableInterface.to_json` defined here
    |
info: This violates the Liskov Substitution Principle

Found 2 diagnostics

Reproduction

uvx ty@0.0.62 check omop_alchemy/

Details

  1. Measurement_Event(ClinicalEvent, Measurement) is defined in omop_alchemy/cdm/handlers/timeline/event_timeline.py:204.
  2. Measurement (via SerialisableTableInterface in orm-loader) declares:
    def to_dict(self, *, include_nulls: bool = False, only: set[str] | None = None, exclude: set[str] | None = None) -> dict[str, Any]: ...
    def to_json(self, **kwargs: Unpack[ToDictKwargs]) -> str: ...
  3. ClinicalEvent overrides both with a narrower signature that accepts none of those keywords:
    def to_dict(self: ClinicalEventProtocol) -> dict[str, Any]: ...
    def to_json(self: ClinicalEventProtocol) -> str: ...
  4. Any code that calls, e.g., measurement_event_instance.to_dict(include_nulls=True) — valid per SerialisableTableInterface's contract, and legal for any other SerialisableTableInterface subclass — raises TypeError: to_dict() got an unexpected keyword argument 'include_nulls' against a Measurement_Event.
  5. Caught statically by ty check omop_alchemy/ as of ty>=0.0.62 (an older/pinned ty doesn't run this check yet, so it's currently invisible in this repo's CI, which pins ty==0.0.61).

Actual behaviour

Measurement_Event.to_dict/to_json silently accept a narrower call signature than the interface they inherit through Measurement. No error at class-definition time; only surfaces as a runtime TypeError if a caller passes a keyword argument the interface promises, or statically via ty>=0.0.62.

Expected behaviour

Measurement_Event (and any other SerialisableTableInterface subclass) should honour the full to_dict/to_json signature it inherits, so it's safely substitutable wherever SerialisableTableInterface is expected. No TypeError on any keyword argument the interface declares.

Error output

error[invalid-method-override]: Base classes for class `Measurement_Event` define method `to_dict` incompatibly
   --> omop_alchemy/cdm/handlers/timeline/event_timeline.py:204:7
    |
204 | class Measurement_Event(ClinicalEvent, Measurement):
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ClinicalEvent.to_dict` is incompatible with `SerialisableTableInterface.to_dict`
    |
   ::: omop_alchemy/cdm/handlers/timeline/event_timeline.py:173:9
    |
173 |     def to_dict(self: ClinicalEventProtocol) -> dict[str, Any]:
    |         ------- `ClinicalEvent.to_dict` defined here
    |
   ::: orm_loader/tables/serialisable_table.py:56:9
    |
 56 |     def to_dict(
    |         ------- `SerialisableTableInterface.to_dict` defined here
    |
info: This violates the Liskov Substitution Principle

error[invalid-method-override]: Base classes for class `Measurement_Event` define method `to_json` incompatibly
   --> omop_alchemy/cdm/handlers/timeline/event_timeline.py:204:7
    |
204 | class Measurement_Event(ClinicalEvent, Measurement):
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ClinicalEvent.to_json` is incompatible with `SerialisableTableInterface.to_json`
    |
   ::: omop_alchemy/cdm/handlers/timeline/event_timeline.py:189:9
    |
189 |     def to_json(self: ClinicalEventProtocol) -> str:
    |         ------- `ClinicalEvent.to_json` defined here
    |
   ::: orm_loader/tables/serialisable_table.py:98:9
    |
 98 |     def to_json(self, **kwargs: Unpack[ToDictKwargs]) -> str:
    |         ------- `SerialisableTableInterface.to_json` defined here
    |
info: This violates the Liskov Substitution Principle

Found 2 diagnostics

System info

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions