Pre-flight
Problem or motivation
docs/index.md/README.md describe omop_graph.oaklib_interface as an "OAK-compliant adapter," but it isn't one yet. oaklib defines roughly 25 separate interface protocols. OMOPAlchemyImplementation (src/omop_graph/oaklib_interface/omop_implementation.py) only subclasses 3 of them, and even within those 3, 28 individual methods currently raise NotImplementedError:
- some stubbed permanently,
- some regressed by a past CDM change with the old implementation left as dead code, and
- at least one left as an explicit unfinished TODO.
The adapter needs to actually become fully oaklib-compliant, matching what the docs already claim.
Proposed solution
Make OMOPAlchemyImplementation fully oaklib-compliant:
- implement every applicable interface and method,
- restore what's currently regressed (
entailed_outgoing_relationships(), entailed_relationships_between(), both broken by a CDM change, old implementation still present as dead code to work from), and
- complete what's left as a TODO (
simple_mappings_by_curie(), backed by OMOP's concept_relationship "Maps to"/"Mapped from" rows).
Where a given interface or method has no meaningful OMOP equivalent, document that explicitly in the code/docs rather than leaving a bare NotImplementedError, so the compliance status is accurate and legible going forward. Update the "OAK-compliant" claim in docs/index.md/README.md to match the result.
Alternatives considered
- Softening the doc claim ("partial OAK support") instead of doing the work. REJECTED, as it doesn't get us a compliant adapter.
- Leaving the 2 regressions and the mapping TODO as-is. REJECTED, as they're part of getting to full compliance, not separable from it.
Pre-flight
Problem or motivation
docs/index.md/README.mddescribeomop_graph.oaklib_interfaceas an "OAK-compliant adapter," but it isn't one yet. oaklib defines roughly 25 separate interface protocols.OMOPAlchemyImplementation(src/omop_graph/oaklib_interface/omop_implementation.py) only subclasses 3 of them, and even within those 3, 28 individual methods currently raiseNotImplementedError:The adapter needs to actually become fully oaklib-compliant, matching what the docs already claim.
Proposed solution
Make
OMOPAlchemyImplementationfully oaklib-compliant:entailed_outgoing_relationships(),entailed_relationships_between(), both broken by a CDM change, old implementation still present as dead code to work from), andsimple_mappings_by_curie(), backed by OMOP'sconcept_relationship"Maps to"/"Mapped from" rows).Where a given interface or method has no meaningful OMOP equivalent, document that explicitly in the code/docs rather than leaving a bare
NotImplementedError, so the compliance status is accurate and legible going forward. Update the "OAK-compliant" claim indocs/index.md/README.mdto match the result.Alternatives considered