Rc 274 passing sites only #7275
Conversation
RoriCremer
force-pushed
the
rc-274-passing-sites-only
branch
from
5174c9d
to
fc359d1
Compare
| fullName="exclude-filtered", | ||
| doc="Don't include filtered sites in the final jointVCF", | ||
| optional=true) | ||
| private boolean XLfiltered = false; |
There was a problem hiding this comment.
typically Java variables start with lower case, (and XL looks like extra large to me!). How about excludeFilteredSites just to be super clear?
There was a problem hiding this comment.
yeah---XL did feel weird---but I wanted to lift it directly from SelectVariants. Is it better to be clear or match the already existing pattern?
| @@ -429,7 +433,7 @@ private void finalizeCurrentVariant(final List<VariantContext> unmergedCalls, | |||
|
|
|||
| // apply VQSLod-based filters | |||
| VariantContext filteredVC = | |||
| noVqslodFilteringRequested ? genotypedVC : filterSiteByVQSLOD(genotypedVC, vqsLodMap, yngMap, performGenotypeVQSLODFiltering); | |||
| noVqslodFilteringRequested ? genotypedVC : filterSiteAlleleByVQSLOD(genotypedVC, vqsLodMap, yngMap, performGenotypeVQSLODFiltering); | |||
There was a problem hiding this comment.
Technically this is filtering at the site level (ie the entire site gets filtered, not a site-allele). If you wanted to make it more specific, maybe filterSiteByAlleleSpecificVQSLOD would be more accurate
| @@ -450,12 +454,15 @@ private void finalizeCurrentVariant(final List<VariantContext> unmergedCalls, | |||
| final VariantContext finalVC = removeAnnotations(filteredVC); | |||
|
|
|||
| if ( finalVC != null ) { | |||
| vcfWriter.add(finalVC); | |||
| // If XLfiltered is true, than the filtered sites should not be added | |||
| if (!XLfiltered || finalVC.isNotFiltered()) { | |||
There was a problem hiding this comment.
this made my brain hurt :D. What do you think of ! (XLfiltered && finalVC.isFiltered). Meh, ok I don't love that either 🤷
There was a problem hiding this comment.
hahaha Andrea listened to me repeat this about 30 times before it was correct. I'm not sure of the best option, but I can comment it more clearly for sure
There was a problem hiding this comment.
is it any clearer if I just swap them?
if (finalVC.isNotFiltered() || !XLfiltered ) { foo }
RoriCremer
force-pushed
the
rc-274-passing-sites-only
branch
from
f5640e3
to
4e2c253
Compare
Locally tested and the exclude-filtered flag removes filtered rows
