Add minimum provenance to FHIR converter for data source #755

robertmitchellv · 2023-08-01T00:09:28Z

PULL REQUEST

Summary

Given a FHIR bundle and an data source add Meta.source to every resource in the bundle indicating a minimum amount of provenance, e.g., "ELR", "ECR", or "VXU". Add this provenance inside of the FHIR converter building block before the response is returned so that downstream building blocks can leverage the provenance.

Related Issue

Fixes #734 #731

one will check that a FHIR bundle is a response.FhirResource and the other will look for LOINC codes and added info the Meta.source

DanPaseltiner · 2023-08-01T13:10:59Z

phdi/fhir/utils.py

+    if is_response_fhirresource(bundle):
+        # Define the specific LOINC codes
+        loinc_ecr = "55751-2"
+        loinc_rr = "88085-6"
+        loinc_elr = "11502-2"
+        loinc_vxu = "60484-3"
+
+        # Iterate through the "entry" list in the FHIR bundle
+        for entry in bundle["response"]["FhirResource"]["entry"]:
+            resource = entry["resource"]
+            if (
+                "type" in resource
+                and "coding" in resource["type"]
+                and len(resource["type"]["coding"]) > 0
+            ):
+                loinc_code = resource["type"]["coding"][0].get("code")
+                if loinc_code == loinc_ecr:
+                    # Insert the new field Meta.source
+                    resource["meta"] = {
+                        "source": "eICR_55751-2_public-health-case-report"
+                    }
+                elif loinc_code == loinc_rr:
+                    # Insert the new field Meta.source
+                    resource["meta"] = {
+                        "source": "RR_88085-6_reportability-response-report"
+                    }
+                elif loinc_code == loinc_elr:
+                    # Insert the new field Meta.source
+                    resource["meta"] = {"source": "ELR_11502-2_laboratory-report"}
+                elif loinc_code == loinc_vxu:
+                    # Insert the new field Meta.source
+                    resource["meta"] = {"source": "VXU_60484-3_cdc-immunization-panel"}
+
+    return bundle


I really appreciate the spirit of what we are doing here. However there are few things I think we need to change.

We aren't going to be able to find provenance data within a FHIR bundle, we need to be able to provide a value for provenance data to this function and have it insert the value in resource.meta.

bundle should simply be a FHIR bundle. The response and FhirResource keys are part of the response object from the FHIR converter, but are not part of the FHIR bundle. The FHIR bundle here is simply the value stored in the response.FhirResource key.

I agree with this and it makes sense.

BradySkylight · 2023-08-01T17:10:32Z

phdi/fhir/utils.py

+        and "resourceType" in bundle["response"]["FhirResource"]
+    ):
+        return bundle["response"]["FhirResource"]["resourceType"] == "Bundle"
+    return False


Where is this anticipated to be used? Are we expecting to always get a FHIR Bundle in an HTTP response?

BradySkylight · 2023-08-01T17:12:08Z

phdi/fhir/utils.py

+        loinc_ecr = "55751-2"
+        loinc_rr = "88085-6"
+        loinc_elr = "11502-2"
+        loinc_vxu = "60484-3"


We may not be able to expect or rely on a proper loinc code to be part of the FHIR bundle's being passed in. It may be better to have a parameter for the 'source_data' that can then be 'filtered' based upon a list of proper/expected results (ie... elr, ecr, etc...) and then add that to the resource.meta.source. Just my thoughts.

…function

BradySkylight

This looks good to me! I like the changes you made

codecov · 2023-08-01T21:02:19Z

Codecov Report

Merging #755 (7330c0f) into main (1a450e8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #755   +/-   ##
=======================================
  Coverage   96.32%   96.32%           
=======================================
  Files          45       45           
  Lines        2557     2557           
=======================================
  Hits         2463     2463           
  Misses         94       94

Flag	Coverage Δ
unit-tests	`96.32% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

…overage

robertmitchellv · 2023-08-01T23:25:53Z

i think this is in a good place now but there's still one matter that needs to be decided before we merge this to main and that relates to where this function should really live.

We can:
A. leave it where it is in the SDK, close this, and move on to the next ticket, where we'll import the function into the fhir converter's app/main.py; or,
B. put it in the fhir converter's app/main.py rather than the SDK

there have been a few conversations about this today in different 1:1 settings with different takes on where it should live so i thought i'd just post here for visibility.

it seems the main reason to go with B is to avoid having to import phdi into that container's dependencies.

BradySkylight · 2023-08-02T00:58:29Z

i think this is in a good place now but there's still one matter that needs to be decided before we merge this to main and that relates to where this function should really live.

We can: A. leave it where it is in the SDK, close this, and move on to the next ticket, where we'll import the function into the fhir converter's app/main.py; or, B. put it in the fhir converter's app/main.py rather than the SDK

there have been a few conversations about this today in different 1:1 settings with different takes on where it should live so i thought i'd just post here for visibility.

it seems the main reason to go with B is to avoid having to import phdi into that container's dependencies.

I vote for B

DanPaseltiner · 2023-08-02T01:06:21Z

i think this is in a good place now but there's still one matter that needs to be decided before we merge this to main and that relates to where this function should really live.
We can: A. leave it where it is in the SDK, close this, and move on to the next ticket, where we'll import the function into the fhir converter's app/main.py; or, B. put it in the fhir converter's app/main.py rather than the SDK
there have been a few conversations about this today in different 1:1 settings with different takes on where it should live so i thought i'd just post here for visibility.
it seems the main reason to go with B is to avoid having to import phdi into that container's dependencies.

I vote for B

Since we are not already importing the SDK in the FHIR converter and it is already a pretty heavy and complex image I agree with @BradySkylight let's go with option B. If we need this functionality somewhere else in the future we can consider a refactor at that point.

…patched in yet

…st data

…cally

DanPaseltiner

LGTM! Thanks for putting this together.

DanPaseltiner · 2023-08-02T00:50:07Z

phdi/fhir/utils.py

+    if data_source == "":
+        raise ValueError(
+            "The data_source parameter must be a defined, non-empty string."
+        )


DanPaseltiner · 2023-08-02T00:50:53Z

phdi/fhir/utils.py

+
+def add_data_source_to_bundle(bundle: dict, data_source: str) -> dict:
+    """
+    Given a FHIR bundle and a a data source parameter the function


Suggested change

Given a FHIR bundle and a a data source parameter the function

Given a FHIR bundle and a data source parameter the function

thanks for catching it! the file moved but i made the change

Robert Mitchell added 2 commits July 31, 2023 15:50

added two functions for minimum provenance

b31ea86

one will check that a FHIR bundle is a response.FhirResource and the other will look for LOINC codes and added info the Meta.source

added test for first of the SDK functions

48fae20

robertmitchellv requested review from emmastephenson, DanPaseltiner and BradySkylight August 1, 2023 00:09

robertmitchellv self-assigned this Aug 1, 2023

DanPaseltiner reviewed Aug 1, 2023

View reviewed changes

BradySkylight reviewed Aug 1, 2023

View reviewed changes

robertmitchellv added 2 commits August 1, 2023 11:23

fixed the SDK function to make it generic; data_source is supplied

6141092

removed test for function we're not using; added test for refactored …

58e0de1

…function

BradySkylight approved these changes Aug 1, 2023

View reviewed changes

robertmitchellv added 2 commits August 1, 2023 12:14

forgot to run black

27c5aed

ensured that there was a 'meta' for every resource; test passes locally

b2ac5a0

robertmitchellv and others added 2 commits August 1, 2023 14:19

added another test to make sure ValueError is raised and to bump up c…

e23bb1c

…overage

Merge branch 'main' into robert/add-data-source-to-bundle

f4a6d97

robertmitchellv changed the title ~~Add minimum provenance to SDK for data source~~ Add minimum provenance to FHIR converter for data source Aug 2, 2023

robertmitchellv added 9 commits August 2, 2023 09:53

removed the functions and tests from SDK

dffa872

added function and test to the fhir-converter script--it hasn't been …

060d181

…patched in yet

removed old import from utils, added import to container tests and te…

b96b29b

…st data

removed pathlib depend and added minimal test bundle

56fed1e

added pytest to test imports

4d1fcfa

added function to the fhir converter function using input_type as source

d9d015c

realized there was already a valid bundle; removed the one i added

2048d65

removed unneeded test data

eddeef6

added logic to test provenance in the fhir converter

5085229

added test for provenance to the valid fhir converter test; passes lo…

d056aa3

…cally

robertmitchellv marked this pull request as ready for review August 2, 2023 21:17

DanPaseltiner approved these changes Aug 2, 2023

View reviewed changes

typo

7330c0f

robertmitchellv merged commit ca1ffbf into main Aug 2, 2023

robertmitchellv deleted the robert/add-data-source-to-bundle branch August 2, 2023 23:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add minimum provenance to FHIR converter for data source #755

Add minimum provenance to FHIR converter for data source #755

robertmitchellv commented Aug 1, 2023 •

edited

Loading

DanPaseltiner Aug 1, 2023

BradySkylight Aug 1, 2023

BradySkylight Aug 1, 2023

BradySkylight Aug 1, 2023

BradySkylight left a comment

codecov bot commented Aug 1, 2023 •

edited

Loading

robertmitchellv commented Aug 1, 2023

BradySkylight commented Aug 2, 2023

DanPaseltiner commented Aug 2, 2023

DanPaseltiner left a comment

DanPaseltiner Aug 2, 2023

DanPaseltiner Aug 2, 2023

robertmitchellv Aug 2, 2023

	Given a FHIR bundle and a a data source parameter the function
	Given a FHIR bundle and a data source parameter the function

Add minimum provenance to FHIR converter for data source #755

Add minimum provenance to FHIR converter for data source #755

Conversation

robertmitchellv commented Aug 1, 2023 • edited Loading

PULL REQUEST

Summary

Related Issue

DanPaseltiner Aug 1, 2023

Choose a reason for hiding this comment

BradySkylight Aug 1, 2023

Choose a reason for hiding this comment

BradySkylight Aug 1, 2023

Choose a reason for hiding this comment

BradySkylight Aug 1, 2023

Choose a reason for hiding this comment

BradySkylight left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 1, 2023 • edited Loading

Codecov Report

robertmitchellv commented Aug 1, 2023

BradySkylight commented Aug 2, 2023

DanPaseltiner commented Aug 2, 2023

DanPaseltiner left a comment

Choose a reason for hiding this comment

DanPaseltiner Aug 2, 2023

Choose a reason for hiding this comment

DanPaseltiner Aug 2, 2023

Choose a reason for hiding this comment

robertmitchellv Aug 2, 2023

Choose a reason for hiding this comment

robertmitchellv commented Aug 1, 2023 •

edited

Loading

codecov bot commented Aug 1, 2023 •

edited

Loading