clingen-data-model / clingen-interpretation

Allele (variant) interpretation model and API for ClinGen
3 stars 1 forks source link

Document that `@id` supplants the allele model's `identifier` #138

Closed bpow closed 6 years ago

bpow commented 6 years ago

Should this refer to the allele registry identifier? (or should the internal ids of our canonical alleles be changed to match that per #135 ?)

If not, does this field need to be here for our examples (since we do not use it)? Or should someone go about filling it in?

bpow commented 6 years ago

Tagging @larrybabb and @cbizon for comment.

larrybabb commented 6 years ago

Great question. I may have overshot adding all the CAR ids into the @id col for our examples. In my opinion, we should use clinvar variation identifiers or car identifiers for the iris or some internally generated iri. Does that help?

larrybabb commented 6 years ago

@bpow In our allele model we defined an "identifier" attribute (as you have pointed out), but in our interpretation model we have switched from the UML class diagram style of defining our concepts to the json-ld RDF-style of defining concepts.
I think that the "identifier" attribute of the CanonicalAllele is equal to the @id attribute that is inherent in every entity in our interpretation concepts.

So my recommendation is that we simply "assume" that @id = identifier (and we remove the explicit "identifier" attribute in our Attribute sheet so as to not confuse things further.

We can make a special notation on the CanonicalAllele page of our documentation that highlights this translation between modeling approaches.

@bpow if you agree please note, open a new issue to remove the "identifier" attribute and annotate the CanonicalAllele page accordingly. You can assign that issue to me.

bpow commented 6 years ago

I'm just going to change the title of this issue to reflect that plan. Off to remove identifier from the attributes table right now...