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

🚧 Map Concepts between KF and FHIR #78

Closed
wants to merge 31 commits into from

Conversation

liberaliscomputing
Copy link
Contributor

This PR is part of an ongoing effort to map Kids First concepts and FHIR resources/profiles. The relevant ticket is #74.


patient = {
'resourceType': 'Patient'
'id': PARTICIPANT.TARGET_SERVICE_ID,
Copy link
Contributor

Choose a reason for hiding this comment

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

target service ID isn't known before it goes into the server. what do we do about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the use of TARGET_SERVICE_ID is like place-holding at the moment. As far as I understand, the 'id' field is the one used when we hit each of the REST endpoints like BASE_URL/Patient/id. So, it seems like we need to define some consistent, global scheme of 'id'. Any thoughts on this, @znatty22 and @ShahimEssaid?

Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

@liberaliscomputing

Couple general comments:

  1. I think each of your python modules should not be named after FHIR base types since they are actually representing the types defined in the Phenopackets FHIR model. So for example, I would get ride of condition.py and rename it to disease.py.

  2. It doesn't look like your payloads are following the FHIR payload strucutre. For example, there are lot of attributes which should NOT be first-level attributes since they are not part of the base model for the entity. They are extensions and therefore should be inside an extensions element in the payload.

  3. Also, it looks like every payload is missing profiles.

Here is an example for disease.py which addresses all of this:

def disease_onset(x):
    """
    https://aehrc.github.io/fhir-phenopackets-ig/ValueSet-onset.html
    """
    pass


def disease_tumor_stage(x):
    """
    https://aehrc.github.io/fhir-phenopackets-ig/ValueSet-tumor-stage.html
    """
    pass


def condition_code(x):
    """
    http://hl7.org/fhir/R4/valueset-condition-code.html
    """
    pass


def body_site(x):
    """
    http://hl7.org/fhir/R4/valueset-body-site.html
    """


disease = {
    'resourceType': 'Condition',
    'meta: {
        'profile': ['http://ga4gh.fhir.phenopackets/StructureDefinition/Individual']
    },
    'id': DIAGNOSIS.TARGET_SERVICE_ID,
    'extension': [
        {
            'text': 'coded-onset',
            'coding': [
                {
                    'system': 'http://foo/coded-onset',
                    'code': 'bar'
                    'display': 'Pediatric onset'

                }
            ]
        },
        {
            'text': 'disease-tumor-stage',
            'coding': [
                {
                    'system': 'http://foo/disease-tumor-stage',
                    'code': 'bar'
                    'display': 'Disease Tumor Stage'

                }
            ]
        }
    ],
    'code': {
        'coding': None,  # condition_code()
        'text':	DIAGNOSIS.NAME
    },
    ...
}

@fiendish
Copy link
Contributor

 'extension': [
         {
             'text': 'CodedOnset',
             'url': 'http://ga4gh.org/fhir/phenopackets/StructureDefinition/CodedOnset',
             'valueCodeableConcept': {
                 'coding': [
                     {
                         'system': 'http://purl.obolibrary.org/obo/hp.owl',
                         'code': 'HP:0410280',
                         'display': 'Pediatric onset'
                     }
                 ]
             }
         }
     ],

doesn't look like

'extension': [
        {
            'text': 'coded-onset',
            'coding': [
                {
                    'system': 'http://foo/coded-onset',
                    'code': 'bar'
                    'display': 'Pediatric onset'

                }
            ]
        },

@znatty22 ?

docs/no_peeking/mappings/biosample.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/disease.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/family.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/individual.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/pedigree_node.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/phenotypic_feature.py Outdated Show resolved Hide resolved
docs/no_peeking/mappings/biosample.py Outdated Show resolved Hide resolved
@znatty22
Copy link
Member

@fiendish @liberaliscomputing

Oops, did that too hastily. The first one is correct.

Co-Authored-By: Natasha Singh <[email protected]>
Co-Authored-By: Natasha Singh <[email protected]>
],
'type': family_type(CONCEPT.PARTICIPANT.SPECIES or constants.SPECIES.HUMAN),
'actual': constants.COMMON.TRUE, # defaults to True
'member': [

Choose a reason for hiding this comment

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

The "member" is missing the "family-member-phenopacket" extension shown here:

https://aehrc.github.io/fhir-phenopackets-ig/StructureDefinition-Family-definitions.html#Group.member.extension:family-member-phenopacket

Also, the new IG doesn't show the ValueSet/CodeSystme that is used in the "family-member-type" extension. I think it was shown in the older IG format.

},
'extension': [
{
'text': 'AffectedStatus',

Choose a reason for hiding this comment

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

Same comment as in other files regarding "text" not being directly under Extension

]
},
{
'text': 'FamilyIdentifier',

Choose a reason for hiding this comment

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

Same

]
},
{
'text': 'IndividualReference',

Choose a reason for hiding this comment

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

Same

'valueReference': {
'reference': f'Individual/{CONCEPT.FAMILY_RELATIONSHIP.PERSON1.TARGET_SERVICE_ID}',
'type': 'Individual',
'identifier': [

Choose a reason for hiding this comment

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

This isn't really necessary if there is a direct reference. See: http://hl7.org/fhir/R4/references-definitions.html#Reference.identifier

'value': CONCEPT.FAMILY_RELATIONSHIP.TARGET_SERVICE_ID
},
{
'value': CONCEPT.FAMILY_RELATIONSHIP.ID

Choose a reason for hiding this comment

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

Add 'system'

'patient': {
'reference': f'Individual/{CONCEPT.FAMILY_RELATIONSHIP.PERSON2.TARGET_SERVICE_ID}',
'type': 'Individual',
'identifier': [

Choose a reason for hiding this comment

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

same as above. not needed if 'reference' is available.

Co-Authored-By: Natasha Singh <[email protected]>
Co-Authored-By: Natasha Singh <[email protected]>
merge functified into dev/map-concepts
fiendish
fiendish previously approved these changes Feb 25, 2020
if subject_id:
retval["subject"] = {
"reference": f"Patient/{individuals[subject_id]['id']}"
"reference": f"Patient/{individuals[subject_id]['id']}",
"type": iRType
Copy link
Contributor

@fiendish fiendish Feb 28, 2020

Choose a reason for hiding this comment

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

what is this type field for? "Patient" is already part of the reference url component. I think if anything the reference field should be f"{iRType}/{individuals[subject_id]['id']}". .

@@ -115,3 +115,4 @@ logsettings.env

# Miscellaneous
.vscode/
.DS_Store
Copy link
Contributor

Choose a reason for hiding this comment

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

Can everything starting with . in this file just be replaced by .* instead?

@znatty22 znatty22 closed this Mar 17, 2020
@fiendish fiendish deleted the dev/map-concepts branch March 17, 2020 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants