-
Notifications
You must be signed in to change notification settings - Fork 594
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
Adds arg to add site level filters to genotype level in ReblockGvcfs #8484
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.
A couple questions, but otherwise looks good!
|
||
final VariantContext filteredRefBlockVC = VariantContextTestUtils.readEntireVCFIntoMemory(output.getAbsolutePath()).getRight().get(0); | ||
Assert.assertFalse(filteredRefBlockVC.isFiltered()); // Ref block is unfiltered even though the input RefBlock and low qual variant were both filtered | ||
Assert.assertFalse(filteredRefBlockVC.getGenotype(0).isFiltered()); // Ref block genotype is also unfiltered |
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.
Why is this what you want?
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.
Hmm, that's a good question. I had assumed that it didn't make sense to have a ref block with a genotype level filter. The low quality variant here was dropped (incorporated into the Ref Block), and I was thinking that the GQ was enough information here that it would only be more confusing if we included the filter in the ref block.
I suppose the outcome of this is that if we kept the filter status in the ref block it would make it into the final VCF at any sites that overlap the entire ref block. So if one variant had a dragen hard filter applied, but then was incorporated into a RefBlock, then all of the 0/0
genotypes would be filtered across the entire ref block, not just the original filtered site. I think this would be more confusing than it's worth, but happy to hear if you think we want to propagate that filter through.
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.
To follow up here: we spoke offline and the conclusion was that GQ0 basically acts as a filter for ref blocks so it's ok to not propagate the filter from the low quality variants.
@@ -175,6 +176,10 @@ public final class ReblockGVCF extends MultiVariantWalker { | |||
@Argument(fullName=KEEP_SITE_FILTERS_LONG_NAME, shortName = KEEP_SITE_FILTERS_SHORT_NAME, doc="Keep site level filters for variants (not ref blocks).") | |||
private boolean keepFilters = false; | |||
|
|||
@Advanced | |||
@Argument(fullName= ADD_FILTERS_TO_GENOTYPE, doc="Add site level filters to genotype level. Site level filters removed by default, if they should be kept, use --" + KEEP_SITE_FILTERS_LONG_NAME) |
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.
Do we already have a test for KEEP_SITE_FILTERS alone?
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.
Yeah, KEEP_SITE_FILTERS was put in earlier and has a test.
No description provided.