VS 396 clinvar grabs too many values #7823
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## ah_var_store #7823 +/- ##
================================================
Coverage ? 51.397%
Complexity ? 26413
================================================
Files ? 2170
Lines ? 164837
Branches ? 17775
================================================
Hits ? 84721
Misses ? 74715
Partials ? 5401 |
RoriCremer
force-pushed
the
rc-vs-396-clinvar-greedy
branch
6 times, most recently
from
616eed2 to
a624d03
Compare
RoriCremer
force-pushed
the
rc-vs-396-clinvar-greedy
branch
from
a624d03 to
52e7468
Compare
gbggrant
approved these changes
May 3, 2022
|
|
||
| def test_clinvar_inclusion(self): | ||
| clinvar_swap = [{ 'id': 'RCV01', \ |
There was a problem hiding this comment.
Do you want to add a test case for a case where the id does NOT start with 'RCV'?
There was a problem hiding this comment.
Yes and I think that line 88 should handle that case
mcovarr
requested changes
May 3, 2022
| @@ -143,6 +143,7 @@ def get_gnomad_subpop(gnomad_obj): | |||
| max_an = None | |||
| max_af = None | |||
| max_subpop = "" | |||
| ## TODO will there ever be unexpected values in gnomad_subpop (values not in gnomad_ordering) ? | |||
There was a problem hiding this comment.
Is this TODO more like "defend against possible unexpected values appearing in gnomad_subpop"?
There was a problem hiding this comment.
yes I think that's a good distinction
| row["clinvar_classification"] = ordered_significance_values # special sorted array | ||
| updated_dates.sort(key=lambda date: datetime.strptime(date, "%Y-%m-%d")) # note: method is in-place, and returns None | ||
| row["clinvar_last_updated"] = updated_dates[-1] # most recent date | ||
| row["clinvar_phenotype"] = sorted(phenotypes) # union of all phenotypes |
There was a problem hiding this comment.
indent inconsistency issues
scripts/variantstore/wdl/extract/create_variant_annotation_table.py
Outdated
Show resolved
Hide resolved
| ordered_significance_values.extend(values_not_accounted_for) # add any values that aren't in significance_ordering to the end | ||
| row["clinvar_id"] = clinvar_ids # array | ||
| row["clinvar_classification"] = ordered_significance_values # special sorted array | ||
| updated_dates.sort(key=lambda date: datetime.strptime(date, "%Y-%m-%d")) # note: method is in-place, and returns None |
There was a problem hiding this comment.
thank you for the comment, I forget that every time I've been away from Python for a while and it never fails to bite me 🙂
…le.py Co-authored-by: Miguel Covarrubias <[email protected]>
…gatk into rc-vs-396-clinvar-greedy
This was referenced Mar 17, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
currently clinvar grabs all values in the variants section of the annotation json---this is wrong because nirvana includes clinvar values from all overlapping variants.
this change limits the clinvar values to the correct variant only
and tests the change as well