clingen-data-model / clingen-interpretation

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

Genetic Condition is a Statement? #126

Closed cbizon closed 6 years ago

cbizon commented 6 years ago

Right now it looks like we're saying that Genetic Condition descends from statement. I assume that this is an error?

larrybabb commented 6 years ago

ummm... isn't everything a statement? sort of?
Should I make it like ContextualAllele?

I could see an argument that GeneticCondition is an Information type, since we are really allowing the user to define the condition using one or more combination of disease/phenotype concepts and/or a self-described name.

cbizon commented 6 years ago

I would say no, that not everything is a statement. As you say, we have things that are domain-related primitives like allele or gene, and I think Condition is one of those. But it is kind of ambiguous.

I guess CanonicalAllele is an analog - there is some provenance about how it got put together, and you could phrase that as an assertion, but it's not the kind of data element that I have in mind when I think about our statements, which are mostly about experimental or observational (or in silico) data.

bpow commented 6 years ago

Actually, at least the mode of inheritance is an assertion-like aspect of GeneticCondition, so unless we added ConditionInheritance as a Statement (which might be a cleaner approach), it does seem like GeneticCondition may be a Statement. I agree that the line is fuzzy, though.

larrybabb commented 6 years ago

I think of the GeneticCondition as a statement because it is some contributor agent's creation of the condition which they are attempting to use to build other statements about. I get that most of the time this will be a single Disease or Phenotype or Disease-inheritance combo, but the idea remains that some agent is deriving this genetic condition to control how they frame the higher level statement which it contributes to.

Is there harm in keeping it as a Statement?

larrybabb commented 6 years ago

I've been thinking about the ClinVar model for interpretation MEASURE(SET) is some SIGNIFICANCE for some TRAIT(SET). In totally ambiguous terms, this statement allows the authoring agent to define a statement about both the MEASURE(SET) and the TRAIT(SET). I understand that totally ambiguous models are not super helpful, but we have learned for sure that we can somewhat scope MEASURE down to a "CanonicalAllele" for our Interpretive Context (sequence variant diagnostic genetic condition interps), but it is also clear we need to allow the community to define their own GeneticConditions (TRAITSETs).

I'll stop now (or for a little while at least)

cbizon commented 6 years ago

We're getting confused, I think, on the difference between Statement and Assertion. It might make sense to label a GeneticCondition as an Assertion, but that doesn't mean it needs to be a Statement.

If you look at the documentation right now and how the nesting works, where would you expect to find genetic condition? I would not expect to find it under statement along with AlleleFrequency and FamilySegregation. But that's where it goes if it's a Statement.

larrybabb commented 6 years ago

I can put it under Assertion (but in SEPIO, assertions are a subclass of Statement).

VariantInterpretation is an Assertion for example but it is currently marked with a parent of "Statement" too.

I think the issue is that we did not split the two subtypes of Statement out explicitly (Assertion and Study Finding).

So all of our Statetments that are either of these point to the same superparent of Statement for now.

I'm fine with decoupling GeneticCondition from the Statement class hierarchy, it just didn't make the most sense based on all the other developments we've been zeroing in on.

Please advise and I'll make the necessary change.

cbizon commented 6 years ago

I think that we do have some questions about the right way to do this, but for now, I'd make it not inherit from Statement or Assertion, but just be a top-level class.

larrybabb commented 6 years ago

ok