-
Notifications
You must be signed in to change notification settings - Fork 6
feat!: update aac 2017 profiles to better reflect guidelines #377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v1
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 I like this fix. This will help me make the ClinVar data work with these statement types.
I'll defer to @ahwagner for the final call on approval.
* Remove Study from StudyStatement models * Add modifiers for classification
I probably should add constraints for the following:
|
TODO: code needs to be tier i-iv, name can have the parentheses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
predicate fix on ClinSigProp.
schema/va-spec/base/json/VariantClinicalSignificanceProposition
Outdated
Show resolved
Hide resolved
schema/va-spec/base/def/VariantClinicalSignificanceProposition.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated descriptions and left some clarifying remarks.
schema/va-spec/base/json/VariantClinicalSignificanceProposition
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex H. Wagner, PhD <[email protected]>
Co-authored-by: Alex H. Wagner, PhD <[email protected]>
Co-authored-by: Alex H. Wagner, PhD <[email protected]>
👍
Okay, I will leave them as we had before (codes and NOT names). As for why we're discussing it on the proposition, I'm not sure, but I can confirm that the codes are defined in the aac profiles.
👍 |
@ahwagner @larrybabb this is ready for re-review |
Refutes should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before I go any further I think we need @ahwagner to verify my assertion below...
Based on our recent discussion on how to represent these AMP/ASCO statements I understood that the top-level statement would be a VariantClinicalSignficianceProposition
-based Statement, which would be profiled under the AMP/ASCO community profies. In addition, we would create three EvidenceLine
profiles for each of the 3 AMP/ASCO propositions related to TherapeuticResponse, Diagnostic and Prognostic, called something like VariantTherapeuticResponseEvidenceLine
, VariantDiagnosticEvidenceLine
and VariantPrognosticEvidenceLine
where each had a targetProposition for the corresponding base profile for TR, DIAG and PROG propositions. These EvidenceLine profiles would reflect the strengths outlined in the AMP/ASCO guidelines for Level A, B, C & D. The would be reverse engineered from ClinVar SCVs with the least amount of lossiness.
Finally, there would also be 3 Statement profiles for each of TR, DIAG and PROG that would also have propositions with the corresponding TR, DIAG and PROG propositions. However, these would be more loosely defined so that the CiVIC strengths could be reflected Levels A/B/C/D/E. These 3 statement profiles would be useful for creating EvidenceItems for the corresponding propoer AMP/ASCO EvidenceLines above.
@ahwagner please verify this as I think @korikuzma may have understood it differently, based on where this PR currently is.
"$ref": "/ga4gh/schema/va-spec/1.0.1/base/json/ExperimentalVariantFunctionalImpactProposition" | ||
}, | ||
{ | ||
"$ref": "/ga4gh/schema/va-spec/1.0.1/base/json/VariantClinicalSignificanceProposition" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we ever need to have the VarClinSigProp ever be a targetProposition on evidenceLine. I think we would only ever have TR, DIAG, PROG props associated at the evidenceLine Level.
- $ref: "/ga4gh/schema/va-spec/1.0.1/base/json/Statement" | ||
- properties: | ||
proposition: | ||
$ref: "/ga4gh/schema/va-spec/1.0.1/base/json/VariantClinicalSignificanceProposition" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be the VariantDiagnosticProposition?
"properties": { | ||
"code": { | ||
"enum": [ | ||
"Strong", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should try to stick to a rule that all enumerations that are used as code
values should be lowercase. We have already set the precedence with definitive
and likely
for strength in the ACMG 2015 profiles, so I think these should be strong
, potential
, definitive
and likely
. If someone wants a proper-cased label then they can use the name
attribute of the MappableConcept.
"properties": { | ||
"code": { | ||
"enum": [ | ||
"Tier I", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These enumerations should be tier I
, tier1
, tier2
, tier3
and tier4
. IMO, roman numerals are for the label and name not the code. We may eventually want to create real dereferenceable codes and put them in a terminology system, but that's a bigger discussion. (and one that is definitely going to happen eventually)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep in mind that the code
is not meant to play the dual role of the label
or name
so we should be careful not to conflate them.
"type": "object", | ||
"properties": { | ||
"name": { | ||
"const": "Tier I (Strong Clinical Significance)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, this should render to Tier I - Strong
which is a precedent that ClinVar set when working with the AMP/ASCO folks to display this data in ClinVar. Having the addition terms "Clinical Significance" is unnecessary and makes the label less compact and useful for displaying in various scenarios.
"type": "object", | ||
"properties": { | ||
"name": { | ||
"const": "Tier II (Potential Clinical Significance)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to Tier II - Potential
"type": "object", | ||
"properties": { | ||
"code": { | ||
"const": "Tier III" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to Tier III - Unknown
per clinvar and their work with the amp/asco folks to depict these type of classifications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be best left as just Tier III
"type": "object", | ||
"properties": { | ||
"code": { | ||
"const": "Tier IV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to Tier IV - Benign/Likely benign
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be best left as Tier IV
since CiVIC will have specific definitive vs likely strength assertions whereas ClinVar will not have this precise info.
close #293
For AAC 2017 community profiles:
StudyStatement
->Statement
Level A-D
->Strong
,Potential
,Definitive
,Likely
Tier I
->Tier I (Strong Clinical Significance)
Tier II
->Tier II (Potential Clinical Significance)
VariantClinicalSignificanceProposition
which the community profiles should use.Diagnostic
,Prognostic
, andTherapeutic Response
Propositions will be used inEvidenceLine.targetProposition
.These changes were needed to support the work I'm doing to submit CIViC data to ClinVar as well as @larrybabb 's work.
@ahwagner Let me know if the description needs to be updated at all