Skip to content

Add study-metadata service to Session - #299

Merged
caiomescouto merged 2 commits into
eu-parc:mainfrom
caiomescouto:study-metadata-service
Oct 20, 2026
Merged

caiomescouto merged 2 commits into
eu-parc:mainfrom
caiomescouto:study-metadata-service

Conversation

@caiomescouto

Copy link
Copy Markdown
Contributor

Implements a new metadata capability following the port/adapter architecture:

  • StudyMetadataConfig (core/models/metadata_dto.py): config DTO with section gates; from_metadata_layout() stub reserves the future peh_model MetadataLayout wire.
  • MetadataInterface (core/interfaces/metadata.py): generic tree-building orchestration (link keys, variable catalogue, datasets, provenance) over the session cache; abstract profile_column / map_data_type primitives implemented by the adapter.
  • DataFrameMetadataAdapter (adapters/metadata/): new "metadata" adapter label; non-identifying column profile (counts only) and wire-format dtype mapping.
  • Session.create_study_metadata(...): public facade method.
  • Population resolved from Study.study_entity_id_list via StudyEntity (StudyPopulation degrades to StudyEntity in the peh model).
  • LOD/LOQ control variables excluded by default; derivation lineage (was_derived_from) walked to include enriched/aggregated observations.
  • Tests: interface unit, adapter unit, Session-level, and an integration test comparing against the notebooks/example golden tree (peh_demo workflow).
  • Docs: session-reference.md (new method + StudyMetadataConfig), adapters.md (metadata area).

Implements a new metadata capability following the port/adapter
architecture:

- StudyMetadataConfig (core/models/metadata_dto.py): config DTO with
  section gates; from_metadata_layout() stub reserves the future
  peh_model MetadataLayout wire.
- MetadataInterface (core/interfaces/metadata.py): generic tree-building
  orchestration (link keys, variable catalogue, datasets, provenance)
  over the session cache; abstract profile_column / map_data_type
  primitives implemented by the adapter.
- DataFrameMetadataAdapter (adapters/metadata/): new "metadata" adapter
  label; non-identifying column profile (counts only) and wire-format
  dtype mapping.
- Session.create_study_metadata(...): public facade method.
- Population resolved from Study.study_entity_id_list via StudyEntity
  (StudyPopulation degrades to StudyEntity in the peh model).
- LOD/LOQ control variables excluded by default; derivation lineage
  (was_derived_from) walked to include enriched/aggregated observations.
- Tests: interface unit, adapter unit, Session-level, and an integration
  test comparing against the notebooks/example golden tree (peh_demo
  workflow).
- Docs: session-reference.md (new method + StudyMetadataConfig),
  adapters.md (metadata area).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Calculation dependencies lose source identity, provenance can break serialization, and missing designs bypass control-variable exclusion.

3 open issues
What changed in this PR

Adds study-metadata generation to Session through the existing port/adapter architecture.

Changes:

  • Adds configurable metadata trees covering study links, variables, datasets, and provenance.
  • Registers a metadata adapter for data-type mapping.
  • Adds documentation and unit/integration coverage.
File Description
tests/​integration/​metadata/​test_study_metadata.py Checks the demo metadata tree.
tests/​integration/​metadata/​input/​peh_demo.yaml Supplies demo metadata resources.
tests/​integration/​metadata/​__init__.py Initializes integration test package.
tests/​core/​session/​test_session_metadata.py Tests Session delegation and configuration.
tests/​core/​interfaces/​metadata/​test_metadata.py Tests metadata assembly and serialization.
tests/​core/​interfaces/​metadata/​__init__.py Initializes interface test package.
tests/​adapters/​metadata/​test_dataframe_adapter.py Tests data-type mapping.
tests/​adapters/​metadata/​__init__.py Initializes adapter test package.
src/​pypeh/​core/​session/​session.py Exposes and registers metadata service.
src/​pypeh/​core/​models/​metadata_dto.py Defines metadata configuration.
src/​pypeh/​core/​interfaces/​metadata.py Builds metadata from cached resources.
src/​pypeh/​adapters/​metadata/​dataframe_adapter.py Maps schema data types.
src/​pypeh/​adapters/​metadata/​__init__.py Initializes metadata adapter package.
Makefile Includes metadata integration tests.
docs/​session-reference.md Documents metadata API and configuration.
docs/​adapters.md Lists metadata adapter support.

Review effort: Balanced

Comment thread src/pypeh/core/interfaces/metadata.py
Comment thread src/pypeh/core/interfaces/metadata.py Outdated
Comment thread src/pypeh/core/interfaces/metadata.py
- Reject non-JSON StudyMetadataConfig.extra_provenance values and
  non-str keys during tree construction with a ValueError naming the
  offending field, instead of breaking json.dumps at serialization
  time; cover caller-provided provenance with serialization tests.
- Classify control observable properties from the dataset schemas in
  addition to the observation designs, so exclude_control_variables
  still holds when an observation design is missing from the cache
  (regression test drops the design from the cache).
- Calculation inputs now carry source_observation next to
  source_variable, matching how execution resolves inputs
  (contextual_field_reference.dataset_label is the observation id);
  the unit test covers two calculations on derived observations that
  share a property, each referencing its own base observation as
  execution requires.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Observation-specific calculations can be misattributed, and accepted numeric subclasses can break YAML serialization.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Normalize numeric subclasses in extra provenance

src/​pypeh/​core/​interfaces/​metadata.py:150

An IntEnum in extra_provenance passes _check_json_value as an integer, but this fallback returns it unchanged. That violates the documented builtin-only output contract and prevents yaml.safe_dump from serializing the accepted value. Normalize integer and float subclasses to builtin types, while preserving booleans.

Medium severity Preserve calculation metadata per observation and property

src/​pypeh/​core/​interfaces/​metadata.py:534

When two reachable observations specify the same observable property with different calculation designs, the global seen set drops the second calculation. _datasets_section then reuses the first calculation for that property's columns in both datasets through structure_by_op, producing incorrect derivation metadata. Preserve calculation metadata per (observation_id, property_id) and use the dataset's observation context when selecting it. Add a regression test with a shared output property and different calculation designs.

3 resolved since last review

Review effort: Balanced

@caiomescouto
caiomescouto merged commit 9b389f8 into eu-parc:main Oct 20, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants