Here are some free-form thoughts on changes I'd like to make as a first pass. These are with the eye towards opening up this repo to be less BCH-specific & more Python-standard.
This is a living issue (i.e. I'll update this description as I dig deeper), that I'm posting for discussion. When/if I file a PR to address this stuff, I'll close this out.
Project stuff
Rename the Python module to chart_review (from chartreview) to better match the package name of chart-review -- this is not critical to me, but I thought it was slightly easier on the eyes
Surprisingly enough, chart-review isn't already taken on PyPI - I thought for sure it would be.
In general, now is a good time to consider our project/module name - do we want cumulus-chart-review? Do we want cumulus-chart-review-review or CCRR? (I might say no to both, but officially putting us under the Cumulus project lets us assume a little more about the metadata in Label Studio -- but cumulus doesn't really need to be part of our name for that to be true)
Add a chart-review CLI to drive the action (so that researchers don't need to open up their Python interpreter -- though folks like you, Andy, can keep doing that)
Reducing BCH-specific stuff
Switch config.py to be a config.yaml or config.json file instead and don't include BCH's version in the repo. This file is site-specific config, and ideally we aren't asking a site to write Python if they don't need to. But everyone knows how to edit a config file.
It's not fully site-specific. The label config is probably study-wide regardless of site. But we don't currently enforce label config in any of our tooling around this. Something to think about for the future.
Looks like CohortReader basically writes this config.json out in the write_config method -- I guess I'm proposing that this file be input rather than output.
mikix
commented
8 months ago
Here are some free-form thoughts on changes I'd like to make as a first pass. These are with the eye towards opening up this repo to be less BCH-specific & more Python-standard.
This is a living issue (i.e. I'll update this description as I dig deeper), that I'm posting for discussion. When/if I file a PR to address this stuff, I'll close this out.
Project stuff
chart_review(fromchartreview) to better match the package name ofchart-review-- this is not critical to me, but I thought it was slightly easier on the eyeschart-reviewisn't already taken on PyPI - I thought for sure it would be.cumulus-chart-review? Do we wantcumulus-chart-review-revieworCCRR? (I might say no to both, but officially putting us under the Cumulus project lets us assume a little more about the metadata in Label Studio -- butcumulusdoesn't really need to be part of our name for that to be true)chart-reviewCLI to drive the action (so that researchers don't need to open up their Python interpreter -- though folks like you, Andy, can keep doing that)Reducing BCH-specific stuff
config.pyto be aconfig.yamlorconfig.jsonfile instead and don't include BCH's version in the repo. This file is site-specific config, and ideally we aren't asking a site to write Python if they don't need to. But everyone knows how to edit a config file.CohortReaderbasically writes thisconfig.jsonout in thewrite_configmethod -- I guess I'm proposing that this file be input rather than output.