Blood Biopsy Annotations and Filters #5317

maddyduran · 2018-10-17T14:27:25Z

an annotation to count the number of Ns at a variant site
a strict strand bias filter
a filter based on the ratio of N count to alt count

codecov-io · 2018-10-17T15:30:23Z

Codecov Report

Merging #5317 into master will increase coverage by 0.001%.
The diff coverage is 89.535%.

@@               Coverage Diff               @@
##              master     #5317       +/-   ##
===============================================
+ Coverage     86.793%   86.794%   +0.001%     
- Complexity     30082     30124       +42     
===============================================
  Files           1842      1843        +1     
  Lines         139291    139480      +189     
  Branches       15358     15376       +18     
===============================================
+ Hits          120895    121060      +165     
- Misses         12810     12827       +17     
- Partials        5586      5593        +7

Impacted Files	Coverage Δ	Complexity Δ
...ute/hellbender/utils/variant/GATKVCFConstants.java	`80% <ø> (ø)`	`4 <0> (ø)`	⬇️
...e/hellbender/utils/variant/GATKVCFHeaderLines.java	`95.783% <100%> (+0.051%)`	`10 <0> (ø)`	⬇️
...ls/walkers/mutect/M2FiltersArgumentCollection.java	`100% <100%> (ø)`	`1 <0> (ø)`	⬇️
...er/tools/walkers/annotator/StrandBiasBySample.java	`86.957% <100%> (+1.242%)`	`9 <2> (+2)`	⬆️
...r/tools/walkers/mutect/Mutect2IntegrationTest.java	`94.369% <100%> (+0.588%)`	`71 <3> (+3)`	⬆️
...te/hellbender/tools/walkers/annotator/CountNs.java	`75% <75%> (ø)`	`8 <8> (?)`
...r/tools/walkers/mutect/Mutect2FilteringEngine.java	`85.232% <76%> (-1.089%)`	`63 <4> (+4)`
...nstitute/hellbender/utils/gcs/BucketUtilsTest.java	`56.303% <0%> (-3.103%)`	`13% <0%> (+1%)`
...ls/genomicsdb/GenomicsDBImportIntegrationTest.java	`86.682% <0%> (-2.388%)`	`77% <0%> (ø)`
...tmutpileup/ValidateBasicSomaticShortMutations.java	`80.172% <0%> (-0.529%)`	`19% <0%> (ø)`
... and 9 more

fleharty

@madduran very minor comments. Back to you.

fleharty · 2018-10-17T15:36:32Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/CountNs.java

+/**
+ * Apply an annotation that reports the number of Ns seen at a given site. This is intended for use on consensus called data.
+ */
+


remove newline

fleharty · 2018-10-17T15:37:06Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/CountNs.java

+
+
+/**
+ * Apply an annotation that reports the number of Ns seen at a given site. This is intended for use on consensus called data.


Should mention here this is a read based annotation, not a fragment based annotation.

fleharty · 2018-10-17T15:40:48Z

...ain/java/org/broadinstitute/hellbender/tools/walkers/mutect/M2FiltersArgumentCollection.java


    public static final String FILTERING_STATS_LONG_NAME = "stats";

+


remove newline here

fleharty · 2018-10-17T15:44:27Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2FilteringEngine.java

@@ -327,6 +327,49 @@ private void applyFilteredHaplotypeFilter(final M2FiltersArgumentCollection MTFA
        }
    }

+    private void applyStrictStrandFilter(final M2FiltersArgumentCollection MTFAC, final VariantContext vc, final FilterResult filterResult) {
+
+        if (! MTFAC.strictStrandBias)  {


extra whitespace between ) {

fleharty · 2018-10-17T15:45:55Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2FilteringEngine.java

+        }
+        final int[] strandBiasCounts = GATKProtectedVariantContextUtils.getAttributeAsIntArray(tumorGenotype, GATKVCFConstants.STRAND_BIAS_BY_SAMPLE_KEY, ()->null, -1);
+
+        final int ALT_FWD_INDEX = 2;


These are constants that should have wider scope. I suggest putting them with other constants around line 22.

fleharty · 2018-10-17T15:54:39Z

src/main/java/org/broadinstitute/hellbender/utils/variant/GATKVCFHeaderLines.java

@@ -77,7 +77,8 @@ private static void addFilterLine(final VCFFilterHeaderLine line) {
        addFilterLine(new VCFFilterHeaderLine(DUPLICATED_EVIDENCE_FILTER_NAME, "evidence for alt allele is overrepresented by apparent duplicates"));
        addFilterLine(new VCFFilterHeaderLine(READ_ORIENTATION_ARTIFACT_FILTER_NAME, "orientation bias detected by the orientation bias mixture model"));
        addFilterLine(new VCFFilterHeaderLine(BAD_HAPLOTYPE_FILTER_NAME, "Variant near filtered variant on same haplotype."));
-
+        addFilterLine(new VCFFilterHeaderLine(STRICT_STRAND_BIAS_FILTER_NAME, "evidence for alt allele is not represented in both directions"));


Capitalize Evidence

fleharty · 2018-10-17T15:54:52Z

src/main/java/org/broadinstitute/hellbender/utils/variant/GATKVCFHeaderLines.java

@@ -77,7 +77,8 @@ private static void addFilterLine(final VCFFilterHeaderLine line) {
        addFilterLine(new VCFFilterHeaderLine(DUPLICATED_EVIDENCE_FILTER_NAME, "evidence for alt allele is overrepresented by apparent duplicates"));
        addFilterLine(new VCFFilterHeaderLine(READ_ORIENTATION_ARTIFACT_FILTER_NAME, "orientation bias detected by the orientation bias mixture model"));
        addFilterLine(new VCFFilterHeaderLine(BAD_HAPLOTYPE_FILTER_NAME, "Variant near filtered variant on same haplotype."));
-
+        addFilterLine(new VCFFilterHeaderLine(STRICT_STRAND_BIAS_FILTER_NAME, "evidence for alt allele is not represented in both directions"));
+        addFilterLine(new VCFFilterHeaderLine(N_RATIO_FILTER_NAME, "ratio of N to alt exceeds specified ratio"));


Capitalize R in ratio

fleharty · 2018-10-17T16:03:31Z

src/main/java/org/broadinstitute/hellbender/utils/variant/GATKVCFHeaderLines.java

@@ -119,7 +120,7 @@ private static void addFilterLine(final VCFFilterHeaderLine line) {
        addInfoLine(new VCFInfoHeaderLine(NON_DIPLOID_RATIO_KEY, 1, VCFHeaderLineType.Float, "Overall non-diploid ratio (alleles/(alleles+non-alleles))"));
        addInfoLine(new VCFInfoHeaderLine(BASE_COUNTS_KEY, 4, VCFHeaderLineType.Integer, "Counts of each base"));
        addInfoLine(new VCFInfoHeaderLine(LOW_MQ_KEY, 3, VCFHeaderLineType.Float, "3-tuple: <fraction of reads with MQ=0>,<fraction of reads with MQ<=10>,<total number of reads>"));
-        addInfoLine(new VCFInfoHeaderLine(N_BASE_COUNT_KEY, 1, VCFHeaderLineType.Float, "Percentage of N bases in the pileup"));
+        addInfoLine(new VCFInfoHeaderLine(N_COUNT_KEY, 1, VCFHeaderLineType.Float, "Percentage of N bases in the pileup"));


Is this a count or a percentage?

should be count

fleharty · 2018-10-17T17:35:51Z

src/test/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2IntegrationTest.java

+        final SAMFileHeader samHeader = M2TestingUtils.createSamHeader();
+        final SAMFileGATKReadWriter writer = M2TestingUtils.getBareBonesSamWriter(samFile, samHeader);
+
+        // create 2 Ts with strand bias


I don't see the 2Ts here.... am I missing something or is this a spurious comment?

that was leftover from before...

fleharty · 2018-10-17T17:37:04Z

src/test/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2IntegrationTest.java

+    }
+
+    @Test
+    public void testLiquidBiopsy() throws Exception {


This isn't really a test of liquid biopsy, could you rename the test?
Something like, testStrictStrandBiasAndN?

maddyduran · 2018-10-17T18:28:00Z

@takutosato or @davidbenjamin
would one of you mind looking at this?

davidbenjamin

Small comments, nothing bigger than a minor refactoring.

davidbenjamin · 2018-10-17T18:34:01Z

...ain/java/org/broadinstitute/hellbender/tools/walkers/mutect/M2FiltersArgumentCollection.java

@@ -113,6 +115,12 @@
    @Argument(fullName = UNIQUE_ALT_READ_COUNT_LONG_NAME, shortName = "unique", optional = true, doc = "Filter a variant if a site contains fewer than this many unique (i.e. deduplicated) reads supporting the alternate allele")
    public int uniqueAltReadCount = 0;

+    @Argument(fullName = N_RATIO_LONG_NAME, optional = true, doc = "Filter a variant if the ratio of Ns in the pileup. A nRatio of 0 will not apply filter.")


"Filter a variant if the ratio of Ns in the pileup _______________"

davidbenjamin · 2018-10-17T18:34:21Z

...ain/java/org/broadinstitute/hellbender/tools/walkers/mutect/M2FiltersArgumentCollection.java

@@ -113,6 +115,12 @@
    @Argument(fullName = UNIQUE_ALT_READ_COUNT_LONG_NAME, shortName = "unique", optional = true, doc = "Filter a variant if a site contains fewer than this many unique (i.e. deduplicated) reads supporting the alternate allele")
    public int uniqueAltReadCount = 0;

+    @Argument(fullName = N_RATIO_LONG_NAME, optional = true, doc = "Filter a variant if the ratio of Ns in the pileup. A nRatio of 0 will not apply filter.")


Just say "A ratio" in the second sentence.

davidbenjamin · 2018-10-17T18:40:57Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2FilteringEngine.java

@@ -27,6 +27,8 @@
    private final Optional<String> normalSample;
    final OverlapDetector<MinorAlleleFractionRecord> tumorSegments;
    public static final String FILTERING_STATUS_VCF_KEY = "filtering_status";
+    private final int ALT_FWD_INDEX = 2;


Rather than explicitly declare array indices here it would be better to have a method like public static int getAltForwardCountsFromFlattenedContingencyTable(final int[] contingencyTable) inside StrandBiasBySample.

davidbenjamin · 2018-10-17T18:46:07Z

src/test/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2IntegrationTest.java

@@ -705,6 +704,80 @@ public void testReadBasedAnnotations() throws IOException {
        Assert.assertFalse(vc.get().getFilters().contains(GATKVCFConstants.STRAND_ARTIFACT_FILTER_NAME));
    }

+
+    public File createSamWithNsandStrandBias(final int numAlts, final int numNs, final int numRefs) throws IOException {


This can be private

davidbenjamin · 2018-10-17T18:47:05Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/CountNs.java

+
+        gb.attribute(GATKVCFConstants.N_COUNT_KEY, Count);
+    }
+    @Override


Put empty lines between methods.

davidbenjamin · 2018-10-17T18:49:09Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/CountNs.java

+    public List<String> getKeyNames() { return Arrays.asList(GATKVCFConstants.N_COUNT_KEY); }
+    private Boolean doesReadHaveN(final GATKRead read, final VariantContext vc) {
+        final int offset = ReadUtils.getReadCoordinateForReferenceCoordinate(read.getSoftStart(), read.getCigar(), vc.getStart(), ReadUtils.ClippingTail.RIGHT_TAIL, true);
+        if (offset == ReadUtils.CLIPPING_GOAL_NOT_REACHED || AlignmentUtils.isInsideDeletion(read.getCigar(), offset)) { return false;}


Why not combine these lines as return offset != ReadUtils.CLIPPING_GOAL_NOT_REACHED && !AlignmentUtils.isInsideDeletion(read.getCigar(), offset) && read.getBase(offset) == 'N'?

maddyduran · 2018-10-17T19:59:27Z

@davidbenjamin back to you

davidbenjamin

Looks good!

takutosato · 2018-10-18T15:00:41Z

...ain/java/org/broadinstitute/hellbender/tools/walkers/mutect/M2FiltersArgumentCollection.java

@@ -119,6 +121,12 @@
    @Argument(fullName = UNIQUE_ALT_READ_COUNT_LONG_NAME, shortName = "unique", optional = true, doc = "Filter a variant if a site contains fewer than this many unique (i.e. deduplicated) reads supporting the alternate allele")
    public int uniqueAltReadCount = 0;

+    @Argument(fullName = N_RATIO_LONG_NAME, optional = true, doc = "Filter a variant if the ratio of Ns in the pileup is greater or equal to this value. A ratio of 0 will not apply filter.")


"Ratio of 0 will not apply the filter" - Is this true?

yeah that's not true anymore, a ratio of 0 will apply the filter to everything
I'm just going to remove this

* adding bloodbiopsy filters/annotations * getting test to pass * adding annotation to variant annotator integration test file * fixing annotation * responding to mark * responding to david * forgot to delete old variable * fixing filter argument description

Madeleine Duran added 4 commits October 16, 2018 15:18

adding bloodbiopsy filters/annotations

7c0632c

getting test to pass

8ecb886

adding annotation to variant annotator integration test file

78a33e9

fixing annotation

3ad5b31

maddyduran self-assigned this Oct 17, 2018

maddyduran requested a review from fleharty October 17, 2018 14:27

fleharty requested changes Oct 17, 2018

View reviewed changes

responding to mark

fcc2b7c

davidbenjamin requested changes Oct 17, 2018

View reviewed changes

madduran added 3 commits October 17, 2018 14:56

Merge branch 'master' into md_bloodbiopsy2

a729a1a

responding to david

4e6add9

forgot to delete old variable

ae50e47

davidbenjamin approved these changes Oct 18, 2018

View reviewed changes

fleharty approved these changes Oct 18, 2018

View reviewed changes

takutosato reviewed Oct 18, 2018

View reviewed changes

fixing filter argument description

4e412ec

maddyduran merged commit f95b6fe into master Oct 18, 2018

maddyduran deleted the md_bloodbiopsy2 branch October 18, 2018 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blood Biopsy Annotations and Filters #5317

Blood Biopsy Annotations and Filters #5317

maddyduran commented Oct 17, 2018

codecov-io commented Oct 17, 2018 •

edited

Loading

fleharty left a comment

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

fleharty Oct 17, 2018

maddyduran Oct 17, 2018

fleharty Oct 17, 2018

maddyduran Oct 17, 2018

fleharty Oct 17, 2018

maddyduran commented Oct 17, 2018

davidbenjamin left a comment

davidbenjamin Oct 17, 2018

davidbenjamin Oct 17, 2018

davidbenjamin Oct 17, 2018

davidbenjamin Oct 17, 2018

davidbenjamin Oct 17, 2018

davidbenjamin Oct 17, 2018

maddyduran commented Oct 17, 2018

davidbenjamin left a comment

takutosato Oct 18, 2018

maddyduran Oct 18, 2018



		/**
		* Apply an annotation that reports the number of Ns seen at a given site. This is intended for use on consensus called data.


		public static final String FILTERING_STATS_LONG_NAME = "stats";

Blood Biopsy Annotations and Filters #5317

Blood Biopsy Annotations and Filters #5317

Conversation

maddyduran commented Oct 17, 2018

codecov-io commented Oct 17, 2018 • edited Loading

Codecov Report

fleharty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maddyduran commented Oct 17, 2018

davidbenjamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maddyduran commented Oct 17, 2018

davidbenjamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Oct 17, 2018 •

edited

Loading