resolving various issues from the code review comments

broadinstitute · Jul 21, 2022 · 07b2b8f · 07b2b8f
1 parent e91d6be
commit 07b2b8f
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 36 deletions.
diff --git a/...in/java/org/broadinstitute/hellbender/tools/walkers/annotator/VariantAnnotatorEngine.java b/...in/java/org/broadinstitute/hellbender/tools/walkers/annotator/VariantAnnotatorEngine.java
@@ -361,7 +361,7 @@ private Map<String, Object> addInfoAnnotations(VariantContext vc, FeatureContext
                 }
             }
         }
-        //TODO this whole thing should be refactored if this is useful or ripped out.
+        //TODO see #7543. This spiderweb of cases should be addressed as part of a more comprehensive refactor of the annotation code with JumboAnnotations.
         if ((fragmentLikelihoods.isPresent() && haplotypeLikelihoods.isPresent()) || readHaplotypeAlleleLikelihoods.isPresent()) {
             jumboInfoAnnotations.stream()
                     .map(annot -> annot.annotate(ref, features, vc, likelihoods,
@@ -389,21 +389,15 @@ private GenotypesContext annotateGenotypes(final ReferenceContext ref,
         final GenotypesContext genotypes = GenotypesContext.create(vc.getNSamples());
         for ( final Genotype genotype : vc.getGenotypes() ) {
             final GenotypeBuilder gb = new GenotypeBuilder(genotype);
-            for ( final GenotypeAnnotation annotation : genotypeAnnotations) {
 
-                genotypeAnnotations.stream().filter(addAnnot)
-                        .forEach(annot -> annot.annotate(ref, vc, genotype, gb, likelihoods));
+            genotypeAnnotations.stream().filter(addAnnot)
+                    .forEach(annot -> annot.annotate(ref, vc, genotype, gb, likelihoods));
 
-                if (fragmentLikelihoods.isPresent() && haplotypeLikelihoods.isPresent()) {
-                    jumboGenotypeAnnotations.stream().filter(addAnnot).forEach(annot ->
-                            annot.annotate(ref, features, vc, genotype, gb, likelihoods, fragmentLikelihoods.get(), haplotypeLikelihoods.get()));
-                }
-
-
-                if (addAnnot.test(annotation)) {
-                    annotation.annotate(ref, vc, genotype, gb, likelihoods);
-                }
+            if (fragmentLikelihoods.isPresent() && haplotypeLikelihoods.isPresent()) {
+                jumboGenotypeAnnotations.stream().filter(addAnnot).forEach(annot ->
+                        annot.annotate(ref, features, vc, genotype, gb, likelihoods, fragmentLikelihoods.get(), haplotypeLikelihoods.get()));
             }
+
             genotypes.add(gb.make());
         }
 

diff --git a/src/testUtils/java/org/broadinstitute/hellbender/testutils/IntegrationTestSpec.java b/src/testUtils/java/org/broadinstitute/hellbender/testutils/IntegrationTestSpec.java
@@ -247,8 +247,9 @@ public static void assertMatchingFiles(final List<File> resultFiles, final List<
             if (isIndex) {
                 if (skipIndexes) {
                     Assert.assertTrue(expectedFile.exists());
+                } else {
+                    Assert.assertTrue(Files.equal(expectedFile, resultFile), "Resulting index file different from expected");
                 }
-                Assert.assertTrue(Files.equal(expectedFile, resultFile), "Resulting index file different from expected");
             } else if (expectedFileName.endsWith(".bam")) {
                 SamAssertionUtils.assertEqualBamFiles(resultFile, expectedFile, compareBamFilesSorted, stringency);
             } else {
@@ -257,28 +258,6 @@ public static void assertMatchingFiles(final List<File> resultFiles, final List<
         }
     }
 
-//    public static void assertMatchingFiles(final List<Path> resultFiles, final ValidationStringency stringency, final boolean trimWhiteSpace, final boolean skipIndexes, final List<Path> expectedFiles) throws IOException {
-//        Assert.assertEquals(resultFiles.size(), expectedFiles.size());
-//        for (int i = 0; i < resultFiles.size(); i++) {
-//            final Path resultFile = resultFiles.get(i);
-//            final String expectedFileName = expectedFiles.get(i).toString();
-//            final Path expectedFile = expectedFiles.get(i);
-//            final boolean isIndex = INDEX_EXTENSIONS.stream().anyMatch(ext -> expectedFileName.endsWith(ext));
-//            if (isIndex) {
-//                //optionally we might not want to make assertions about the exact bytes of the index files
-//                if (skipIndexes) {
-//                    Assert.assertEquals(java.nio.file.Files.readAllBytes(resultFile).length > 0, java.nio.file.Files.readAllBytes(expectedFile).length > 0, String.format("Resulting index file %s was missing or empty",expectedFileName));
-//                } else {
-//                    Assert.assertEquals(java.nio.file.Files.readAllBytes(resultFile), java.nio.file.Files.readAllBytes(expectedFile), String.format("Resulting index file %s different from expected", expectedFileName));
-//                }
-//            } else if (expectedFileName.endsWith(".bam")) {
-//                SamAssertionUtils.assertEqualBamFiles(resultFile, expectedFile, null, stringency);
-//            } else {
-//                assertEqualTextFiles(resultFile, expectedFile, null, trimWhiteSpace);
-//            }
-//        }
-//    }
-
     public static void assertEqualTextFiles(final File resultFile, final File expectedFile) throws IOException {
         assertEqualTextFiles(resultFile, expectedFile, null, true);
     }