Add trans-spec generator (replaces make_yaml)#299
Open
amc-corey-cox wants to merge 6 commits intomainfrom
Open
Add trans-spec generator (replaces make_yaml)#299amc-corey-cox wants to merge 6 commits intomainfrom
amc-corey-cox wants to merge 6 commits intomainfrom
Conversation
Clean rewrite of DMCYAML_07_GenerateYAML_forPy.py as a single parameterized function. Saved here while the scaffold branch keeps the original for collaborative walkthrough.
- Pull in updated yaml_measobs.j2 from RTIInternational/NHLBI-BDC-DMC-HV PR #542: single-quoted expressions (valid YAML), conditional visit/age handling, renamed unit_casestmt_custom field - Add fillna(0) to match upstream script's NaN handling - Update sample CSV with new columns (has_visit, has_visit_expr, associatedvisit_expr, has_age, unit_casestmt_custom) - Expand tests from 6 to 16: YAML validity, all 4 unit branches, visit/age conditionals, common field mapping
- Rename src/dm_bip/make_yaml/ to src/dm_bip/trans_spec_gen/ - Add `dm-bip generate-trans-specs` Typer subcommand with --input, --output, --cohort, --entity, --template options - Rewrite module README with usage, input CSV format, output structure - Add trans_spec_gen to top-level README repo structure
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #299 +/- ##
==========================================
+ Coverage 65.63% 66.28% +0.64%
==========================================
Files 5 6 +1
Lines 227 264 +37
==========================================
+ Hits 149 175 +26
- Misses 78 89 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR brings the trans-spec (LinkML-Map transformation spec) YAML generation workflow into dm-bip by adding a dedicated trans_spec_gen module and a dm-bip generate-trans-specs Typer CLI command, backed by a Jinja2 template and unit tests.
Changes:
- Added
dm_bip.trans_spec_genmodule (generator, template, and module-level README) to generate per-variable YAML trans-spec files from a metadata CSV. - Added
dm-bip generate-trans-specsCLI command to run the generator. - Added unit tests + synthetic CSV fixture to validate output structure and key template branches; removed the old
make_yamlplaceholder README.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/dm_bip/trans_spec_gen/generate_trans_specs.py |
Implements CSV→Jinja2→YAML file generation. |
src/dm_bip/trans_spec_gen/templates/yaml_measobs.j2 |
Provides the MeasurementObservation YAML template with unit/visit/age branching. |
src/dm_bip/trans_spec_gen/README.md |
Documents CLI usage, CSV schema, and output layout. |
src/dm_bip/cli.py |
Adds the generate-trans-specs CLI command wiring. |
tests/unit/test_generate_trans_specs.py |
Adds unit tests covering template branches and file output. |
tests/input/make_yaml/shortdata_sample.csv |
Adds a synthetic CSV fixture for tests. |
src/dm_bip/make_yaml/README.md |
Removes the deprecated/placeholder module README. |
README.md |
Updates repo structure listing to include trans_spec_gen/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix missing closing paren in uuid5 visit expression template - Fix missing unit derivation in unit_expr template branch - Sanitize bdchm_varname with Path.name to prevent path traversal - Fix stale module docstring in tests - Tighten unit_expr and visit_expr test assertions
Sigfried
requested changes
Apr 14, 2026
Collaborator
Sigfried
left a comment
Sigfried
left a comment
There was a problem hiding this comment.
You're not going to include the rest of the Stata pipeline here? When we convert the Stata code to Python, it would probably be good not to have to go back and forth between repos.
|
|
||
| ## Input CSV format | ||
|
|
||
| The metadata CSV must contain these columns: |
Collaborator
There was a problem hiding this comment.
It currently expects these specific columns because it only generates specs for MeasurementObservation, right? Should we make notes here or anywhere about how this will be changing when we generalize to other domains?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/dm_bip/trans_spec_gen/dm-bip generate-trans-specsCLI command (Typer) for generating LinkML-Map transformation specification YAML from a metadata CSVmake_yaml/module totrans_spec_gen/Test plan
uv run dm-bip generate-trans-specs --helpand with sample data)Closes #175