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

Packaging mode for studies #323

Merged
merged 4 commits into from
Dec 10, 2024
Merged

Packaging mode for studies #323

merged 4 commits into from
Dec 10, 2024

Conversation

dogversioning
Copy link
Contributor

-Adds a new cli param to the build argument, --prepare, which renders queries instead of execute them
-Adds a fixture to replace individual instances of clearing os environment variables

Checklist

  • Consider if documentation in docs/ needs to be updated
    • If you've changed the structure of a table, you may need to run generate-md
    • If you've added/removed core study fields that not in US Core, update our list of those in core-study-details.md
  • Consider if tests should be added
  • Update template repo if there are changes to study configuration in manifest.toml

cumulus_library/cli.py Outdated Show resolved Hide resolved
Comment on lines +96 to +97
prepare: bool = False,
data_path: pathlib.Path | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: here and in build_matching_files, I might drop any default argument. Because you pass in prepare and data_path everywhere you call these, and you want to avoid a future bug where a new call to these gets introduced but the dev forgets to pass down the CLI arg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving this in at the main entrypoint since a lot of tests assume it's there, but i've pulled everything downstream

cumulus_library/cli_parser.py Outdated Show resolved Hide resolved
cumulus_library/cli_parser.py Outdated Show resolved Hide resolved
cumulus_library/actions/builder.py Outdated Show resolved Hide resolved
cumulus_library/actions/builder.py Outdated Show resolved Hide resolved
cumulus_library/actions/builder.py Outdated Show resolved Hide resolved
cumulus_library/actions/builder.py Outdated Show resolved Hide resolved
cumulus_library/actions/builder.py Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Dec 10, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
2364 2356 100% 90% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
cumulus_library/actions/builder.py 100% 🟢
cumulus_library/actions/file_generator.py 100% 🟢
cumulus_library/builders/valueset/umls.py 100% 🟢
cumulus_library/cli.py 100% 🟢
cumulus_library/cli_parser.py 100% 🟢
TOTAL 100% 🟢

updated for commit: 944be91 by action🐍

@dogversioning dogversioning merged commit 763e0de into main Dec 10, 2024
3 checks passed
@dogversioning dogversioning deleted the mg/packaging branch December 10, 2024 15:07
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

Successfully merging this pull request may close these issues.

2 participants