-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add back VariantClassification
to disinguish from VariantStudyStatement
#211
Comments
This sounds correct to me; thanks for digging into this and recommending this. To add to this, I'm not aware of any datasets that would require |
Hi @korikuzma Just dropping a line to let you know I see this, but am currently focused on the MAVE Profile work. I will share my thoughts/proposals for how I think the SEPIO-VA model might handle this in the next couple weeks, if that is OK. |
@mbrush maybe it would make sense for @korikuzma to propose a solution here so we aren't gated by your availability? In VRS we often pass off issues arising from implementations to the reporting product developers. |
Absolutely. I didn’t mean to imply that a proposal can’t be drafted in the meantime. Just that there’s a history of discussion and ideas around modeling CIViC data using SEPIO/VA that I wanted to review in light of recent developments, and share for consideration. I’ll drop a few thoughts and references here over the next couple days, and we can discuss/compare notes as your model drafts develop. As for the idea of bringing back some intermediate abstract classes, happy to revisit this if you all think it is useful. But we’ll want to be careful that this has general utility beyond the immediate use cases, and doesn’t impede broader utility of the models and framework. |
Hi @korikuzma - I updated some of our earlier CIViC data example diagrams to reflect outcomes of recent VA profiling work - in particular ongoing MAVE Functional Impact profile efforts - and copied some relevant examples into the slides here. These might be useful to consult as you draft additional profiles - in particular how content of a CIViC EID record might get split between VA Evidence Line and StudyStatement objects. Feel free to reach out to me with questions. We will be discussing/finalizing the MAVE Functional Impact profile work on a call Tuesday 10-22 at 2pm PT / 5pm ET. You are welcome to attend this call if Alex thinks it would be useful for you. Thanks! |
@mbrush in your example, couldn't implementers still incorrectly add |
Yes - the questions here are (1) whether these are constraints we want in a Standard Profile that will need to serve some adopters for which they may be too restrictive; and (2) if so, whether defining various abstract Statement subtypes as you propose is the best way to achieve this - given the consequences/implications this would have. I don't know the answer here - just posing the questions. |
For my use case, I need to be able to represent CIViC evidence and assertions. I don't think we have a model that makes this distinction.
Back in March, we had
VariantClassification
andVariantStudySummary
to represent CIViC assertions and evidence respectively:va-spec/schema/va-spec/core-im/core-im-source.yaml
Lines 214 to 237 in 4c14e9f
It would be nice to do something similar again. We'd want to enforce having
hasEvidenceLines
andclassification
inVariantClassification
, but these fields would not exist inVariantStudySummary
. Both models would still inherit fromStatement
.@mbrush @larrybabb @ahwagner thoughts?
The text was updated successfully, but these errors were encountered: