-
Notifications
You must be signed in to change notification settings - Fork 28
Testing external contributor PR #313 #315
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
Open
PlanXCyborg
wants to merge
32
commits into
master
Choose a base branch
from
copy-of-external-pr-313
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
fdc70f1
Added cli/fhir_transform.py and transform_utils.py
dsafarian 134df13
Add fhir_transform cli, utils, and tests
dsafarian 92d603e
Remove unnecessary imports
dsafarian 44940a9
Add documentation, format tests, integrate into gen3 cli, add test_da…
dsafarian aab4d54
Fix and add tests, create folder even if output_file path doesn't exist
dsafarian c66e855
Pass all tests, modify multiprocessing to not cause issues with gen3,…
dsafarian 404be39
test md
dsafarian 8899bef
Add fhir.md
dsafarian 2d2e6fe
Merge branch 'uc-cdis:master' into fhir_transform
dsafarian 3f861f6
Black formatting
dsafarian 80f1315
black formatting
dsafarian d8ae042
Fix pull request comments
dsafarian 76c6797
Delete test outputs
dsafarian a34ed6c
Remove setuptools dependency from pyproject.toml
dsafarian d929fd2
Fix pyproject.toml dependencies and poetry.lock
dsafarian 8e0174c
Remove FIXME
dsafarian 6192216
Fix default working directory and directory creation, imports, rename…
dsafarian 6b5f319
Remove test outputs, fix typos, add assertions, change to SHA-256 has…
dsafarian d00ecf5
Fix hash bug
dsafarian 8339418
Add unit test for global authz and fix tmp_path
dsafarian 9cbf8ba
Comment out all parrallelization code
dsafarian 8f5eee5
Remove _resume_run from all code
dsafarian 9cdf886
Fix exception handling
dsafarian 818c537
Fix resource_type check
dsafarian dd2abb2
Use fhirpathpy.compile instead of fhirpathpy.evaluate for speed up.
dsafarian 157eb94
Add check for if output file exists and is not empty for _is_new.
dsafarian d7773f6
Fix poetry dependencies
dsafarian 82e8710
Remove all parallelization code and make fhir import optional in cli
dsafarian 9c42dd7
Add helper function for tag_fhir_resources_with_authz and fix tmp_roo…
dsafarian a9a0418
Black formatting
dsafarian 7b942b0
fix(drsclient): remove dep due to out of date sub-deps, implement DRS…
Avantol13 e7c6dac
Merge branch 'master' into copy-of-external-pr-313
Avantol13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ## FHIR | ||
|
|
||
| This integration aims to enhance the data ingestion capabilities of Gen3 by integrating a Fast Healthcare Interoperability Resources (FHIR) data ingestion pipeline & tools. FHIR is an important standard for working with Electronic Health Records (EHR) and we have started development of a Gen3 FHIR Proxy service. The overall goal is to allow users to seamlessly ingest data into an existing FHIR server. Gen3 is working on adding support for FHIR and this tool will help with data preparation and interaction in the future. | ||
|
|
||
|
|
||
| The fhir commands can be invoked as follows | ||
|
|
||
| `gen3 fhir COMMAND [ARGS] [OPTIONS]` | ||
|
|
||
| For a list of commands and options run | ||
|
|
||
| `gen3 fhir --help` | ||
|
|
||
| For example, the following tags the 'Patient.ndjson' file with Gen3 authorization and outputs 'gen3_Patient.ndjson' using the authorization rules from 'config.yaml' | ||
|
|
||
| `gen3 fhir transform Patient.ndjson gen3_Patient.ndjson config.yaml --batch_size 10000 --workers 8 ` | ||
|
|
||
|
|
||
| The authorization configuration file has to be in yaml format and can have multiple conditions, e.g: | ||
|
|
||
| ```yaml | ||
| rules: | ||
| - resource_type: "Patient" | ||
| condition: "Patient.managingOrganization.reference = 'Organization/site-alpha'" | ||
| authz: "/programs/Alpha/projects/Main" | ||
|
|
||
| - resource_type: "Patient" | ||
| condition: "Patient.managingOrganization.reference = 'Organization/site-alpha' and Patient.gender = 'male'" | ||
| authz: "/programs/Alpha/projects/Main" | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| import click | ||
| import os | ||
| import pathlib | ||
| from cdislogging import get_logger | ||
| from gen3.fhir import * | ||
|
|
||
| logging = get_logger(__name__) | ||
| TMP_ROOT = pathlib.Path(".fhir_transform/tmp") | ||
|
|
||
|
|
||
| @click.group() | ||
| def fhir(): | ||
| """Commands for FHIR data processing""" | ||
| pass | ||
|
|
||
|
|
||
| @click.command( | ||
| context_settings={"help_option_names": ["-h", "--help"]}, | ||
| help="Tag Bulk FHIR data with Gen3 compatible authorization tags", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add more detail here about the input_file, what type - what should be in it, etc |
||
| ) | ||
| @click.argument( | ||
| "input_file", | ||
| type=click.Path(exists=True, dir_okay=False, readable=True), | ||
| metavar="input_file", | ||
| ) | ||
| @click.argument( | ||
| "output_file", type=click.Path(dir_okay=False, writable=True), metavar="output_file" | ||
| ) | ||
| @click.argument( | ||
| "config", | ||
| type=click.Path(exists=True, dir_okay=False, readable=True), | ||
| metavar="config", | ||
| ) | ||
| @click.option( | ||
| "-b", | ||
| "--batch_size", | ||
| type=click.IntRange(min=1), | ||
| default=10000, | ||
| show_default=True, | ||
| metavar="batch_size", | ||
| help="batch size for chunking", | ||
| ) | ||
| @click.option( | ||
| "-w", | ||
| "--workers", | ||
| type=click.IntRange(min=1), | ||
| default=8, | ||
| show_default=True, | ||
| metavar="workers", | ||
| help="number of parallel processes", | ||
| ) | ||
| @click.option( | ||
| "--force", | ||
| is_flag=True, | ||
| help="Remove all intermediate files for this run before exiting even if run crashes", | ||
| ) | ||
| def cli( | ||
| input_file: str | os.PathLike[str], | ||
| output_file: str | os.PathLike[str], | ||
| config: str | os.PathLike[str], | ||
| workers: int, | ||
| batch_size: int, | ||
| force: bool, | ||
| ): | ||
| """ | ||
| CLI implementation of fhir_tagger. | ||
|
|
||
| Args: | ||
| input_file (str): Input .ndjson file | ||
| output_file (str): Output file name | ||
| config (str): .yaml file with authorization rules | ||
| batch_size (int): numb er of lines per chunk | ||
| workers (int): number of parallel processes | ||
| force (bool): remove all intermediate files for this run before exiting even if it crashes | ||
| """ | ||
| fhir_tagger(input_file, output_file, config, workers, batch_size, force) | ||
|
|
||
|
|
||
| @click.command( | ||
| context_settings={"help_option_names": ["-h", "--help"]}, | ||
| help="Remove all intermediate files in the tmp folder from previous runs", | ||
| ) | ||
| @click.option( | ||
| "--dry-run", | ||
| is_flag=True, | ||
| help="Used with --cleanup, report what would be deleted with --cleanup without deleting the files", | ||
| ) | ||
| @click.option( | ||
| "--force", | ||
| is_flag=True, | ||
| help="Remove temporary directory ignoring status of each directory", | ||
| ) | ||
| def cleanup(dry_run: bool, force: bool): | ||
| """ | ||
| Remove all intermediate files in the tmp folder from previous runs | ||
|
|
||
| Args: | ||
| dry_run (bool): If True, list the files that would be removed, but not actually remove them | ||
| force (bool): Delete all intermediate directories disregarding the status | ||
| """ | ||
|
|
||
| cleanup_fhir_transform_artifacts(dry_run=dry_run, force=force) | ||
|
|
||
|
|
||
| fhir.add_command(cli, name="transform") | ||
| fhir.add_command(cleanup, name="cleanup") | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this example fails to load b/c of Patient matching 2 authorization conditions, let's have spearate blocks to show each and have a single, good config someone could copy and use right away. And actually an example file or link to ones used in tests would be good too