search

phenopackets / phenopacket-schema

Repository for the GA4GH phenopacket schema
https://phenopacket-schema.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
74 stars 28 forks source link

Add Treatment History #59

Closed julesjacobsen closed 3 years ago

julesjacobsen commented 5 years ago

Could be important for future computational cancer cases

pnrobinson commented 5 years ago

This is quite complicated and should be saved for v2 following a requirements analysis!

julesjacobsen commented 4 years ago

Have added in a quick sketch of a Timecourse message. How a Treatment fits with this has yet to be decided - should it be a set of repeated elements within a Phenopacket, or something separate?

https://github.com/phenopackets/phenopacket-schema/blob/a8938ab16cf031df453ad317b1789767ebe944b9/src/main/proto/phenopackets.proto#L92-L100

cmungall commented 4 years ago

Hmm, this feels a bit awkward, but I get how it involves minimal disruption: It allows sets of phenopackets to be collected together in one document, which is better than forcing them into separate documents.

But we have a lot of repetition of the same information in each 'observation'

Is this proposed as an alternative to the plan to extend PhenotypicFeature to allow time periods?

Minor: So if a phenopacket is really an observation, shouldn't the class be 'Observation'?

pnrobinson commented 4 years ago

Repetition of information is not unknown in this space (c.f. our favorite clinical standard). Given that many elements of Phenopackets are not required, software that generates phone-time courses would not have to create repetitive messages. I would love to get your take as to the best place to put the treatment message.

julesjacobsen commented 4 years ago

Minor: So if a phenopacket is really an observation, shouldn't the class be 'Observation'?

Yes! Observation is the name I've been searching for! Pity we went with Phenopacket, but at least its snappy.

cmungall commented 4 years ago

OK, so if a phenopacket can be interpreted as an observation, then I think we do need an over-arching object to collect these. I don't like TimeCourse as a name, but I'm OK thinking of this as a set of observations (or PhenopacketSet)

The decision to inline certain objects like gene, subject inside the observation is a little odd, since we end up repeating this (makes sense retrospectively when our use case was 1 observation, but does it make sense prospectively).

If we do go with this repetition, things get a bit odd. E.g. consider a phenopacketSet about the same individual over a couple of years. We would repeat the same individual in each nested phenopacket, with the same id, the same values, except varying the age each year. I presume we would make it invalid to change other values about that individual (how do we encode this?).. except maybe phenotypic sex...?

IMO it seems cleaner to make phenopacket a semantics-free data holder, have a single individual object, and to include timestamps on any diachronic property - phenotypicfeature, phenotypic sex, age, tests, disease, ...

I don't really know how fields like genes and variants should be interpreted outside the original genetic disease use case

pnrobinson commented 3 years ago

This is now included in the v1.1 version. See MedicalAction and other referenced messages https://github.com/phenopackets/phenopacket-schema/blob/f73b4bd5ad966321d3d6b3c7ff8aa8f63820941b/src/main/proto/base.proto#L561

I will close this issue, see many related issues about treatment for further discussion.