ga4gh-beacon / beacon-v2-Models

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

individual-MID-example is not updated to the draft4 #66

Closed redmitry closed 2 years ago

redmitry commented 2 years ago

Hello,

I see the age.json has changed the format between draft3 and draft4.

Age:
      type: object
      properties:
        age:
          type: string
          example: 'P32Y6M1D'
        ageGroup:
          type: string
          example: 'NCIT:C27954 (Adolescent)'

vs (new):

{
  "title": "Age",
  "type": "object",
  "properties": {
    "iso8601duration": {
      "type": "string",
      "example": "P32Y6M1D"
    }
  }
}

Example files must be updated accordingly: individual-MID-example.json

Current schema(s): Individual

{
  "title": "Individual",
  "type": "object",
  "properties": {
    "diseases": {
      "type": "array",
      "items": {
        "$ref": "../common/disease.json"
      }

Disease

{
  "title": "Disease",
  "type": "object",
  "properties": {
    "ageOfOnset": {
      "$ref": "./age.json"
    },

Cheers,

Dmitry

Tom-Shorter commented 2 years ago

It might make sense to change ageOfOnset (not just in disease) to reference one of age.json or ageRange.json as well as fixing the example, @redmitry, @mbaudis do you agree?

redmitry commented 2 years ago

It might make sense to change ageOfOnset (not just in disease) to reference one of age.json or ageRange.json as well as fixing the example, @redmitry, @mbaudis do you agree?

The only place where "ageOfOnset" property is defined is in the disease.json and it is "$ref": "./age.json". The only thing that is needed the change is the example.

mbaudis commented 2 years ago

ageOfOnset compares to the Phenopackets onset and should be a timeElement, where age and ageGroup then represent options.

redmitry commented 2 years ago

OIC. On the other hand timeElement is quite generic. Be more specific vs more Phenopackets, I am afraid I am not qualified to opinion here.

mbaudis commented 2 years ago

There you go https://github.com/ga4gh-beacon/beacon-v2-Models/pull/77 . Does this work? @redmitry @Tom-Shorter

redmitry commented 2 years ago

The age is defined as:

{
  "properties": {
    "iso8601duration": {
      "type": "string",
    }
  }
}

So the example

 "examples": [
        { "ageGroup": { "id": "NCIT:C49685", "label": "Adult 18-65 Years Old" }, "age": "P32Y6M1D"},
        { "ageRange": { "start": "P18Y", "end": "P59Y" } },
        { "age": "P2M4D"}
      ]

is invalid, should be:

{ "ageRange": { "start": {"iso8601duration": "P18Y"}, "end": {"iso8601duration": "P59Y" }} },
{ "age":  {"iso8601duration": "P2M4D"}}
mbaudis commented 2 years ago

@redmitry Well caught - and there are (were) more of these... Fixed in https://github.com/ga4gh-beacon/beacon-v2-Models/commit/9b74a7e8b17ae8724d837451ad26d21662c3baa6, I hope?!

redmitry commented 2 years ago

I think it is ok now.