Skip to content
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

discuss: change CLINVAR id from RCV to Variant ID #69

Merged
merged 3 commits into from
Aug 17, 2021
Merged

Conversation

colleenXu
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jul 19, 2021

Pull Request Test Coverage Report for Build 1140665613

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.831%

Totals Coverage Status
Change from base Build 1137355909: 0.0%
Covered Lines: 439
Relevant Lines: 444

💛 - Coveralls

@newgene
Copy link
Member

newgene commented Jul 28, 2021

@colleenXu what's the justification to switch to using ClinVar variant_id instead of RCV id? Is that the primary ID for Clinvar in Biolink model? Just need a description to document why we made this change.

I also suggest to remove dbnsfp.clinvar.clinvar_id, clinvar field should be the canonical source for ClinVar.

@colleenXu
Copy link
Contributor Author

colleenXu commented Jul 28, 2021

@newgene There are many identifiers in Clinvar, and it's not clear to me which ID the biolink-model is referring to with the CLINVAR prefix. See the id_prefixes here (wait...there's CLINVAR and ClinVarVariant? What are these?).

Based on my reading on Clinvar identifiers and this FAQ on RCV, it seemed that RCV were more like an association's ID (phenotype/disease + variant).

So I thought maybe a different ID from clinvar refers to the SequenceVariant itself and should be used...

According to the linked Clinvar doc on identifiers, the closest to a "unique variant ID" is maybe a "VariationID" or "AlleleID"?

@newgene
Copy link
Member

newgene commented Jul 28, 2021

OK, if this does not link to a known issue or expected use case, I would hold off this one.

We are making changes to ClinVar source at MyVariant.info now: biothings/myvariant.info#127. Will reevaluate this after this myvariant issue is closed.

@colleenXu colleenXu changed the title discuss: change CLINVAR id from RCV to Variant ID wait for myvariant changes, discuss: change CLINVAR id from RCV to Variant ID Jul 28, 2021
@colleenXu
Copy link
Contributor Author

colleenXu commented Aug 17, 2021

Based on the conversation in NCATSTranslator Slack, the CLINVAR ID prefix for SequenceVariant refers to the "Variation ID" in Clinvar.

My recommendation is to go forward with this PR, once I add a commit to remove the dbnsfp field mapping and resolve merge conflicts...

I've confirmed that the clinvar.variant_id field in MyVariant corresponds to the Clinvar Variation ID. For example, see how the clinvar.variant_id:596318 corresponds to the record here that states that the Clinvar Variation ID is also 596318

@colleenXu colleenXu changed the title wait for myvariant changes, discuss: change CLINVAR id from RCV to Variant ID discuss: change CLINVAR id from RCV to Variant ID Aug 17, 2021
@newgene newgene merged commit b68d0fc into main Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants