search

clingen-data-model / clingen-interpretation

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

remove "label" attributes since anything can have a label #193

Closed bpow closed 6 years ago

bpow commented 6 years ago

Related to #192

The following Types have a "label" attribute:

Entity id Attribute id Entity name
E40 A072 DomainEntity
E39 A130 UserLabel
E54 A132 ValueSet
E41 A146 IdentifierSystem
E12 A149 Agent

These will are being removed, with a new __labels sheet to associate ids with labels, regardless of Type (or even in absence of Type)

bpow commented 6 years ago

Note that UserLabel.label had been declared with cardinality 1..1, others with cardinality 0..1.

I don't know if we want to keep these constraints, and if so how we can verify them under this way of representing labels (e.g., not attached to class structure).

larrybabb commented 6 years ago

Great call out. It is interesting that HL7 FHIR's docs deal with this in the following way.

When they are defining Resources (clusters of classes around one core concept - Patient, DiagnosticReport, etc...). They model the classes and define all the attributes as if they would all be available in any given implementation. Since they are essentially defining an "umbrella" model to cover many different use cases, they end having to make almost everything optional (0..1 or 0..*) with the exception of protocol specific values (like id, and there are some other exceptions).

However, when an implementation guide is created the "real" constraint for a given application is then applied to this "extension" or "profile" such that all users of the same implementation guide can rely on a more appropriately constrained model.

Since we are not really separating our generalized model from our implementation, we are sort of in a jam here. I think it is useful to show tech folks on our tech pages what the general cardinality is. I also think we should write some doc at the top of our Tech page stack that points out that further constraints may be helpful when using these resources in a given use case.

Not sure if that helps much since everything we have been targeting is a single use case, one all encompassing fully detailed variant interpretation. Maybe if we had another use case or two that utilized Variant Interpretation in different context's or ways then it would be easier to see the need for separating our baseline docs with our implementation docs.

This will most definitely happen once we start into other forms of assertions (I think).

bpow commented 6 years ago

Re: @larrybabb 's comment...

If we do want to restrict cardinality, then the type system seems like the right place to do that-- right now we are talking about having a separate validation for valuesets to determine if something is an instance of a valueset. That bit of "protocol" at least is an orthogonal concern to the type system. I think it would be confusing to have a protocol somehow specify different cardinalities for attributes outside of the type system, but maybe I don't understand what you are getting at.

bpow commented 6 years ago

I'm at a stopping point for this...

The __labels sheet is created and there is code in reformat_examples.rb to handle it.

Still to fix: things that are now of type @iri are still listed in the DomainEntity sheet (so they end up rendered as having 'type': 'DomainEntity'), and some of those things have other attributes besides 'label' (Families) have a "description", a few ascertainments have userLabels associated.

so far, delta T for this issue is about 90 minutes.

larrybabb commented 6 years ago

value set labels has been moved to the __label sheet (and updated to the latest names)