Valuset builder #298
Valuset builder #298
Conversation
This makes the following changes: - Removed map of deprecated 'statistics' module to 'builders' (breaking) - Added a builder for constructing clinical valusets - Light restructuring of PSM builder due to structure change - Adjusted CTAS from parquet creation for better cross-DB compatibility
| expansion_rules_path = ( | ||
| pathlib.Path(__file__).resolve().parents[0] / "expansion_rules.tsv" | ||
| ) | ||
| # TODO: get this table to write its parquet to the valueset_data folder |
There was a problem hiding this comment.
I'm just seeing this note to myself on review - i'll look into this
| @@ -19,8 +19,7 @@ | |||
| log_utils, | |||
| study_manifest, | |||
| ) | |||
| from cumulus_library.builders import protected_table_builder | |||
| from cumulus_library.statistics import psm | |||
| from cumulus_library.builders import protected_table_builder, psm_builder, valueset_builder | |||
There was a problem hiding this comment.
nit: your own life might be better if you re-exported all the individual builder classes in builders/__init__.py, so that you could do the following:
from cumulus_library import builders
builder = builders.PsmBuilder()
This also avoids some churn as you add/remove/rename builders.
(I've wound up on this pattern in the ETL anyway - all the cumulus_etl.* modules are toplevel concepts like "loaders" or "nlp" and then those modules are little fiefdoms that export their internals out to the top level. Helps me mentally map the code structure too, to keep semantic meaning one level deep. I don't always hit that mark, but I try 😄)
There was a problem hiding this comment.
i'm slowly coming around to this view of things - i may do it in a separate PR for my own sanity.
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
dogversioning
force-pushed
the
mg/valueset
branch
from
8b727f0
to
4d01bad
Compare
This makes the following changes:
Checklist
docs/needs to be updatedgenerate-mdcorestudy fields that not in US Core, update our list of those incore-study-details.mdmanifest.toml