ga4gh-beacon / beacon-v2-Models

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

Procedure.ageAtProcedure suspicious type #35

Closed redmitry closed 3 years ago

redmitry commented 3 years ago

Hello,

The ageAtProcedure type in the Procedure is defined as "$ref": "./timeElement.json" what looks suspicious as the timeElement.json is not an age.

Dmitry

mbaudis commented 3 years ago

@redmitry That's from Phenopackets v2. The TimeElement was introduced as a general wrapper for different ways to express anything "time". So an age - like in ageAtProcedure - in principle could be an ontology class representing "adult" or a range (P18Y - P64Y) etc.

Now I guess the implementation may not be correct; it probably should be a oneOf construct... Suggestions welcome!

redmitry commented 3 years ago

Now I guess the implementation may not be correct; it probably should be a oneOf construct... Suggestions welcome!

I see. It depends whether you want to keep Phanopackets v2 structure.

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "TimeElement",
  "description": "Definition of a wrapper for various time descriptors.",
  "type": "object",
  "$comments": "TODO: Add other values from https://github.com/phenopackets/phenopacket-schema/blob/v2/docs/time-element.rst",
  "definitions": {
    "TimeElement": {
      "oneOf": [
        {"$ref": "./age.json"},
        {"$ref": "./ageRange.json"}
      ]
    }
  }
}

Currently, all "common" types are defined as objects (wrappers). Age is an object with the property "iso8601duration" so, doing something like:

"ageAtProcedure": {
  "$ref": "./timeElement.json#/definitions/TmeElement"
 }

will lead to the json:

"ageAtProcedure": {
  "iso8601duration": "P32Y6M1D"
}

Compare to the phenopackets example:

timeElement:
    age:
        iso8601duration: "P25Y"

Anyway at least I see the rationale of the structure (phenopackets), thank you very much for the clarification.

Dmitry