Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial robustification thoughts #1

Closed
mikix opened this issue Oct 11, 2023 · 1 comment
Closed

Initial robustification thoughts #1

mikix opened this issue Oct 11, 2023 · 1 comment

Comments

@mikix
Copy link
Contributor

mikix commented Oct 11, 2023

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
Copy link
Contributor Author

mikix commented Oct 13, 2023

Solved by #2

@mikix mikix closed this as completed Oct 13, 2023
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

No branches or pull requests

1 participant