search

ga4gh-beacon / beacon-v2-Models

Models that leverage the Beacon Framework v2
Apache License 2.0
4 stars 7 forks source link

Should Treatment have an ageAtOnset? #90

Closed julesjacobsen closed 2 years ago

julesjacobsen commented 2 years ago

https://github.com/ga4gh-beacon/beacon-v2-Models/blob/539cb05dada755c7b1a66b2e187b9668fcfa670b/BEACON-V2-Model/common/treatment.json#L24-L26

Copy/Paste error?

mbaudis commented 2 years ago

@julesjacobsen This had been there before the last update, as the only time element. Not sure if there is a remaining case; in principle one can have one for relative treatment start dates etc.; but I'm not sure if this is a theoretical case or is being requested...

julesjacobsen commented 2 years ago

OK, just checking - seemed out of place and possibly redundant given the DoseInterval requires a start and end period.

mbaudis commented 2 years ago

Well, there is a general problem with time points which are relative to the age of an individual (i.e. you know how old they were when getting the disease, and then treatment for ... months etc., but no dates). Not really solved...

jrambla commented 2 years ago

I'm asking Claudia about that property...

clauw87 commented 2 years ago

Agree with mbaudis

Now in "interval" we have as examples the start and ends as date format ( "start": "1967-11-11T07:30:00+01", "end": "1967-11-18T12:00:00+01" )instead of age at´ in ISO duration format as it used to be in a previous version, so there is really not redundance here since the full birthdate of Individual is not in the Individual model. Not sure when this changed from ageAt to timestamp though and why.

However in the model documentation here http://docs.genomebeacons.org/schemas-md/obj/treatments/ interval description seems to fit the timestamp idea ("interval Time interval with start and end defined as ISO8601 time stamps.") but inside the properties "start" and "end" there is still the property type as iso8601duration. So there seems to be some confusion there.

If timestamp, no redundancy, if ISO duration ad ´ageAtstart´ of course there will be.

mbaudis commented 2 years ago

@clauw87 Can you have a look at https://github.com/ga4gh-beacon/beacon-v2-Models/pull/116 ? The problem was that we didn't "Phenopackify" deep enough. If one uses the PXF way (with complete timeElement), then this is covered (and we follow a common way to do this).

mbaudis commented 2 years ago

Closing this since now discussed elsewhere.