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

Support a flat ndjson dir layout as well as a hierarchical one #244

Merged
merged 2 commits into from
May 29, 2024

Conversation

mikix
Copy link
Contributor

@mikix mikix commented May 29, 2024

Previously, --load-ndjson-dir would only look for an ETL output-style format like this:

dir/condition/*.ndjson
dir/patient/*.ndjson

But now it will also look for flat files as well (i.e. ETL input-style format) like this:

dir/1.Patient.ndjson
dir/Patient.october.ndjson
dir/Patient.ndjson

This will make it nicer to use the --load-ndjson-dir flow when you are working on ndjson files directly, without going through Cumulus ETL first.

Checklist

  • Consider if documentation (like in docs/) needs to be updated
  • Consider if tests should be added
  • Update template repo if there are changes to study configuration

Previously, --load-ndjson-dir would only look for an ETL output-style
format like this:

dir/condition/*.ndjson
dir/patient/*.ndjson

But now it will also look for flat files as well (i.e. ETL input-style
format) like this:

dir/1.Patient.ndjson
dir/Patient.october.ndjson
dir/Patient.ndjson

This will make it nicer to use the --load-ndjson-dir flow when you are
working on ndjson files directly, without going through Cumulus ETL
first.
Copy link
Contributor

@dogversioning dogversioning left a comment

Choose a reason for hiding this comment

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

I think this needs some documentation tweaks for the public site (mostly here: https://docs.smarthealthit.org/cumulus/library/creating-studies.html#testing-studies), but otherwise seems totally reasonable.

@mikix
Copy link
Contributor Author

mikix commented May 29, 2024

I think this needs some documentation tweaks for the public site

Added a new commit to update that page (the only doc page that mentioned --load-ndjson-dir) to basically drop the ETL step entirely.

@mikix mikix merged commit 27438b3 into main May 29, 2024
3 checks passed
@mikix mikix deleted the mikix/more-ndjson branch May 29, 2024 17: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