Spike: design refactor of remaining modules in schema dir

smart-on-fhir / cumulus-library

https://docs.smarthealthit.org/cumulus/library/

Apache License 2.0

2 stars 0 forks source link

Spike: design refactor of remaining modules in schema dir #240

Open dogversioning opened 1 month ago

dogversioning commented 1 month ago

The remaining contents of the schema directory pre-date the cutover to things like python builders, and aren't used currently in any live study code (though they :might: be used in study development). We already have one example of this work being re-created, so we may want to bite the bullet and try and provide these things as a more standard API with documentation.

Things to consider in the redesign:

what things belong in python? what things should be left in sql/ETL-created tables?
How much do we need to re-enforce FHIR typing?
should they be class based, module based, or some combination?
How would this (or wouldn't this) interface with some programmatic definition of where data in a study was sourced from?

mikix commented 1 month ago

I think it's also worth being somewhat comfortable with duplication. Like, in the linked example of the metrics study having its own list of some systems.

What if the metrics study starts caring about some new system that Library doesn't care about? Should it blend the new system on top of some Library code or just wait for a change to land upstream and then depend on the new Library?

Just you know, there's a slider between duplication and dependency and it's not always obvious where the best spot is.

Does the Library have any public API at all now? It would be nice to have some rules of thumbs for what the Library wants to take over from / offer to studies, as we enter the world of being an actual (lowercase) library.

dogversioning commented 1 month ago

Does the Library have any public API at all now? It would be nice to have some rules of thumbs for what the Library wants to take over from / offer to studies, as we enter the world of being an actual (lowercase) library.

This is really the question I'm asking here, I think. Right now we have a handful of method/classes that are sort of orphans in that they're not under test or anything like that, but they are imported in some of the studies ( covid, though that typesystem is not used in the code on trunk, and opioid). We could decide these should be a public API, or we could reconsider what to do with them.

It's fine, maybe even desirable for studies to maintain their own list - but if that's where it is, we should consider pruning these from core, if they're not used there.

mikix commented 1 month ago

My bias has been towards treating studies as little black boxes with as few touch points with the rest of the system (just for archivability / reproducability of studies later). In my head, it would be neat if the cumulus-library of ten years from now could still run today's studies. But that might be a fool's errand / more effort than it's worth.

So that bias is pushing me towards Library options like:

not providing any python code / sql templates for studies
provide code/sql templates designed to be copied into studies, but not accessible via import - embrace the duplication but still help authors
providing versioned code/templates (when we make a new breaking change, we copy old stuff into a v2/ folder and keep providing v1 too -- just like we're a web service providing versioned rest apis)

Or... just not caring so much. That thing about running old studies is just something in my head, and not yet a project goal I don't think - maybe it should be / maybe not.