Adding argument to GenotypeGVCFs to keep only RAW_GT_COUNT #7996

meganshand · 2022-08-22T14:12:04Z

Currently, there is an argument to keep all raw annotations, but the flow based pipeline has a use case that needs to keep RAW_GT_COUNT without keeping the other raw annotations. This new argument allows the user to keep RAW_GT_COUNT while still cleaning up the other raw annotations in GenotypeGVCFs.

…aw annotations)

codecov · 2022-08-22T14:39:32Z

Codecov Report

Merging #7996 (f7b8aa9) into master (05a7634) will increase coverage by 7.336%.
The diff coverage is 89.796%.

Additional details and impacted files

@@               Coverage Diff               @@
##              master     #7996       +/-   ##
===============================================
+ Coverage     79.301%   86.637%   +7.336%     
- Complexity     36613     38937     +2324     
===============================================
  Files           2328      2335        +7     
  Lines         181684    182658      +974     
  Branches       19950     20052      +102     
===============================================
+ Hits          144077    158250    +14173     
+ Misses         30805     17370    -13435     
- Partials        6802      7038      +236

Impacted Files	Coverage Δ
...efaultGATKVariantAnnotationArgumentCollection.java	`100.000% <ø> (ø)`
.../annotator/allelespecific/ReducibleAnnotation.java	`100.000% <ø> (ø)`
...er/tools/walkers/GenotypeGVCFsIntegrationTest.java	`90.653% <81.818%> (+89.216%)`	⬆️
...titute/hellbender/tools/walkers/GenotypeGVCFs.java	`93.243% <88.889%> (-1.672%)`	⬇️
...ools/walkers/annotator/VariantAnnotatorEngine.java	`87.259% <92.308%> (+0.214%)`	⬆️
...ine/GATKPlugin/GATKAnnotationPluginDescriptor.java	`79.310% <100.000%> (+0.363%)`	⬆️
...ers/GenotypeGVCFsAnnotationArgumentCollection.java	`100.000% <100.000%> (ø)`
...e/hellbender/utils/variant/GATKVCFHeaderLines.java	`95.385% <100.000%> (ø)`
...hellbender/tools/walkers/annotator/RawGtCount.java	`63.415% <0.000%> (-12.195%)`	⬇️
...hellbender/tools/walkers/sv/CollectSVEvidence.java	`74.888% <0.000%> (-4.990%)`	⬇️
... and 253 more

samuelklee · 2022-08-22T17:02:22Z

As discussed, let's quickly see if we can generalize this to allow one to specify a set of raw annotations to keep. If it looks like that will be a reach, just let me know and I can go ahead and review. Thanks!

gatk-bot · 2022-08-24T19:51:44Z

Github actions tests reported job failures from actions build 2921600692
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
integration	11	2921600692.12	logs
integration	8	2921600692.0	logs

meganshand · 2022-08-25T18:29:01Z

@samuelklee I made an attempt to make the argument more general. It should be ready for review. Thanks!

samuelklee · 2022-08-29T20:47:34Z

Thanks, @meganshand! Just a heads up, I’m mostly out this week since it’s a vacation week for our daycare. But will try to sneak this in if I can.

samuelklee

Thanks @meganshand! Sorry for the delay. Some minor changes, and some potentially major/breaking ones that might merit more discussion.

If you need this merged quickly, I'll trust you to make some of the corresponding decisions. But feel free to ping me if you'd like to discuss!

samuelklee · 2022-09-08T15:04:36Z

src/test/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFsIntegrationTest.java

@@ -930,7 +928,8 @@ public void testRawGtCountAnnotation() {
        args.addReference(b37_reference_20_21)
                .addVCF(reblockedGVCF)
                .addOutput(output)
-                .add("keep-combined-raw-annotations", true)


Note that this replaces the previous test in favor of this new one. Perhaps we should consider just adding a new test, so that we cover both 1) keeping all combined annotations, and 2) keeping only the specified raw annotations?

We've now made these two arguments (keep all combined and keeping only specified) mutually exclusive (not at the time of your review, but since then based on other comments). So I'm going to leave this test as is.

Sorry, just to be clear, you mean you'll revert this test to its original behavior, right? In that case, will you add another test for the keep-specified behavior?

The original behavior of --keep-combined-raw-annotations set to true while --keep-specified-raw-annotations is not empty is no longer allowed due to the mutex. So I think I am not reverting this test to the original behavior. Does that make sense? This test sets --keep-specified-raw-annotations to RawGtCount

Sorry, by "original behavior" I mean the current behavior of the test as it is in master. This test sets --keep-combined-raw-annotations true and hence additionally retains RAW_MQandDP.

In contrast, you modify the test in your branch so that only RAW_GT_COUNT is retained and we check that RAW_MQandDP is not. This means the code path that is responsible for retaining RAW_MQandDP is no longer run in this test, at least (and is perhaps not run for this particular input test file in any test at all). Even though the original test does not explicitly check for any particular RAW_MQandDP output, this is still an effective drop in test coverage.

Probably not anything to sweat about, but that's all that I wanted to point out in my original comment. I'll leave it up to you to decide if --keep-combined-raw-annotations true is sufficiently covered by the other tests so that we don't have to worry about replacing the original test here.

Oh! Sorry, I see what you're saying now. I think I originally added this test as a way of testing RawGTCount, rather than --keep-combined-raw-annotations true, so I overlooked that it did actually test the latter too. I do think the other tests sufficiently cover --keep-combined-raw-annotations true but I very much see how that is not clear from this change.

samuelklee · 2022-09-08T15:23:48Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+    /**
+     * Keep only the specific combined raw annotations specified (removing the other raw annotations if keep-combined-raw-annotations is not set).
+     */
+    @Argument(fullName= KEEP_SPECIFIED_RAW_ANNOTATION_LONG_NAME, shortName = KEEP_SPECIFIED_RAW_ANNOTATION_SHORT_NAME, optional = true,


Should we perhaps use mutex to ensure we don't use both -keep-raw and -keep-combined simultaneously?

Do you have an idea of the usefulness of -keep-combined (i.e., the convenience of being able to keep all combined raw annotations, even if we don't know what those annotations might be) and where it might be currently used (e.g., WARP WDLs, etc.)? How much of a pain would it be to require the use of -keep-raw in all cases---presumably one would always be able to know which annotations one wants to keep?

In any case, -keep-raw and -keep-combined (and the corresponding long names) don't really give the sense of -keep and -keep-all that is implemented here. Assuming we have the freedom to muck with the name of -keep-combined, do you think it might be worth changing? And should we also change the short/long names of -keep-raw to somehow include combined (as you note in the Javadoc here)?

A general comment: as a relative neophyte to this annotation framework, I will say that I find the lack of clear documentation/definitions for terms like "combined" and "raw" makes it hard to be sure what these arguments (or even the tool) are doing, especially from the doc strings and tool docs alone. Given that you have more experience with this framework, do you feel the same, or do you think things are clear as is?

Happy to hash all of this out further offline/elsewhere, seems a bit thorny!

I don't know if -keep-combined is used anywhere currently. I'm assuming it's mostly useful for debugging/development, but I don't know. I would rather not break backwards compatibility though, so I think we should still leave it in and I have a preference to not rename it.

I can add some more documentation for these arguments though to help make it clearer. I agree that the names don't well represent what they are doing.

samuelklee · 2022-09-08T15:31:30Z

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

        }
        variantOverlapAnnotator = initializeOverlapAnnotator(dbSNPInput, featureInputs);
        reducibleKeys = new LinkedHashSet<>();
        useRawAnnotations = useRaw;
        keepRawCombinedAnnotations = keepCombined;
+        for (String rawAnnot : rawAnnotationsToKeep) {
+            if (!variantAnnotationKeys.contains(rawAnnot)) {
+                throw new UserException("Requested --" + GenotypeGVCFs.KEEP_SPECIFIED_RAW_ANNOTATION_LONG_NAME + ": " + rawAnnot + " is not available. Add requested annotation with --" + StandardArgumentDefinitions.ANNOTATION_LONG_NAME + ".");


It's confusing that the command line looks something like: --keep-specific-raw-annotation: RAW_GT_COUNT --annotation RawGtCount (since the latter is derived from the Java class name). Is this pattern something that could be cleaned up at this stage, or has that ship long sailed?

And this exception message, while helpful, doesn't actually tell you what value you need to supply to the annotation argument---not sure if that's actually that trivial to look this up and output it?

This is finally resolved! I got help from @cmnbroad to add an argument that uses the GATKAnnotationPlugin, but even so I think I did not implement this in the cleanest way. @cmnbroad I'd appreciate if you'd take a look at this branch again to make sure I didn't miss anything.

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

samuelklee · 2022-09-08T15:41:26Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+     */
+    @Argument(fullName= KEEP_SPECIFIED_RAW_ANNOTATION_LONG_NAME, shortName = KEEP_SPECIFIED_RAW_ANNOTATION_SHORT_NAME, optional = true,
+            doc="Keep only the specific combined raw annotations specified (removing the other raw annotations).")
+    protected List<String> keepSpecifiedRawAnnotations = new ArrayList<>();


Depending on what you do with -keep-combined above, you may want to add additional argument documentation/validation/sanitization here. E.g., add to the doc string that duplicate values will be ignored, and ensure that downstream code enforces/respects this as early as possible.

samuelklee · 2022-09-08T15:46:57Z

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

@@ -253,6 +274,14 @@ public Map<String, Object> combineAnnotations(final List<Allele> allelesList, Ma
    public VariantContext finalizeAnnotations(VariantContext vc, VariantContext originalVC) {
        final Map<String, Object> variantAnnotations = new LinkedHashMap<>(vc.getAttributes());

+        //save annotations that have been requested to be kept
+        final Map<String, Object> savedRawAnnotations = new LinkedHashMap<>();
+        for(String rawAnnot : rawAnnotationsToKeep) {


Same comment about final here. Note that it's actually used in other loops in this method...up to you if you want to make it consistent everywhere.

samuelklee · 2022-09-08T15:53:27Z

src/test/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFsIntegrationTest.java

@@ -942,6 +941,7 @@ public void testRawGtCountAnnotation() {
                Assert.assertEquals(rawGtCount.get(0), ".");


What's going on with the hom-ref count? I see the TODO in the RawGtCount class but I don't understand the fundamental limitation.

Incidentally, perhaps we can go ahead and expand the header-line documentation of this annotation to explicitly give the hom-ref/het/hom-var order. Maybe this is obvious to everyone else, but why not document it for dummies like me?

The issue is that RawGtCount is combined by GenomicsDB as a sum. There is no RawGtCount annotation on RefBlocks so the HomRef count always ends up being 0 in this case. It seems like ExcessHet isn't actually using RawGtCount as I expected. It recalculates the counts in GenotypeUtils.computeDiploidGenotypeCounts which doesn't seem to use RawGtCount so I'm not sure why RawGtCount is the rawKey associated with ExcessHet. Seems like this whole thing could be cleaned up so that we use the same method ExcessHet does to calculate the count of Het and HomVar genotypes at a site, but I'm not sure why we didn't do that in the first place.

Also I added the documentation to the header line.

gatk-bot · 2022-09-26T16:10:35Z

Github actions tests reported job failures from actions build 3129008425
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
integration	11	3129008425.12	logs
integration	8	3129008425.0	logs

gatk-bot · 2022-09-30T17:49:54Z

Github actions tests reported job failures from actions build 3160429275
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
integration	11	3160429275.12	logs
integration	8	3160429275.0	logs

meganshand · 2022-09-30T17:55:28Z

I think tests are failing only because I changed a header line. I will update the expected VCFs.

cmnbroad

@meganshand Some changes requested, hope they make sense. I didn't really look at the VariantAnnotatorEngine changes, but presumably someone else is reviewing those.

cmnbroad · 2022-10-04T18:37:02Z

...dinstitute/hellbender/cmdline/GATKPlugin/DefaultGATKVariantAnnotationArgumentCollection.java

@@ -82,4 +81,7 @@ public boolean getDisableToolDefaultAnnotations() {
    public boolean getEnableAllAnnotations() {
        return enableAllAnnotations;
    }
+
+    @Override
+    public List<String> getKeepSpecifiedCombinedAnnotationNames() { return Collections.emptyList(); }


Since the new argument is specific to GenotypeGVCFs and GenotypeGVCFsAnnotationArgumentCollection, we definitely don't want to add this method here. It should only be in GenotypeGVCFsAnnotationArgumentCollection. See my comments on GenotyeGVCFs elsewhere for more details on how to make this unnecessary.

cmnbroad · 2022-10-04T20:19:37Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+        GATKReadFilterPluginDescriptor readFilterDescriptor = new GATKReadFilterPluginDescriptor(getDefaultReadFilters());
+        return useVariantAnnotations()?
+                Arrays.asList(readFilterDescriptor, new GATKAnnotationPluginDescriptor(
+                        new GenotypeGVCFsAnnotationArgumentCollection(),


In the branch I gave you, I had instantiated GenotypeGVCFsAnnotationArgumentCollection directly like this, but it should really be a GenotypeGVCFs instance variable. Then you can just pass the instance variable to the annotation plugin here, and reference it below when you need to get call getKeepSpecifiedCombinedAnnotationNames.

Also, make sure that when you add the instance variable, you don't annotate it as an @ArgumentCollection (and maybe even add a comment saying that @ArgumentCollection is left out on purpose) since if you do the parser will complain about duplicate arg names. i.e., just add:

// @ArgumentCollection deliberately omitted since this is passed to the annotation plugin
final GenotypeGVCFsAnnotationArgumentCollection genotypeGVCFsAnnotationArgs = new GenotypeGVCFsAnnotationArgumentCollection();

cmnbroad · 2022-10-04T20:21:41Z

...in/java/org/broadinstitute/hellbender/cmdline/GATKPlugin/GATKAnnotationPluginDescriptor.java

+        return this.allDiscoveredAnnotations;
+    }
+
+    public GATKAnnotationArgumentCollection getUserArgs() {


getUserArgs can also be removed from this class (see the comments elsewhere).

cmnbroad · 2022-10-04T20:46:26Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+        final Collection<Annotation>  variantAnnotations = makeVariantAnnotations();
+        final GATKAnnotationPluginDescriptor pluginDescriptor = getCommandLineParser().getPluginDescriptor(GATKAnnotationPluginDescriptor.class);
+        final List<String> annotationStringsToKeep = pluginDescriptor.getUserArgs().getKeepSpecifiedCombinedAnnotationNames();


If you add an instance variable of type GenotypeGVCFsAnnotationArgumentCollection called genotypeGVCFsAnnotationArgs (or something) as suggested above, then you can access that directly here, instead of asking the plugin descriptor for it. It will have it's actual type (as opposed to the base class type GATKAnnotationArgumentCollection), and you'll be able to call the GATKAnnotationArgumentCollection::getKeepSpecifiedCombinedAnnotationNames method on it directly. Then the getKeepSpecifiedCombinedAnnotationNames method can be removed from the GATKAnnotationArgumentCollection base class; and getUserArgs can be removed from the plugin descriptor. Only GenotypeGVCF's will know anything about the new arg collection or the new method:

Suggested change

final Collection<Annotation> variantAnnotations = makeVariantAnnotations();

final GATKAnnotationPluginDescriptor pluginDescriptor = getCommandLineParser().getPluginDescriptor(GATKAnnotationPluginDescriptor.class);

final List<String> annotationStringsToKeep = pluginDescriptor.getUserArgs().getKeepSpecifiedCombinedAnnotationNames();

final Collection<Annotation> variantAnnotations = makeVariantAnnotations();

final GATKAnnotationPluginDescriptor pluginDescriptor = getCommandLineParser().getPluginDescriptor(GATKAnnotationPluginDescriptor.class);

final List<String> annotationStringsToKeep = genotypeGVCFsAnnotationArgs.getKeepSpecifiedCombinedAnnotationNames();

cmnbroad · 2022-10-04T21:17:46Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+        final Map<String, Annotation> allDiscoveredAnnotations = pluginDescriptor.getAllDiscoveredAnnotations();
+        final Set<Annotation> annotationsToKeep = annotationStringsToKeep.stream().map(allDiscoveredAnnotations::get).collect(Collectors.toSet());


I think it would be preferable to add an override of makeVariantAnnotations into this class (adjacent to the getPluginDescriptors override), and move the computation of which annotations to keep into that, rather than inlining it here.

Also, you're basically giving priority to the new arg in the case where it conflicts with, say, an exclude arg, which I think is fine if thats what you want, but you might want to document that as part of the new arg if thats what you go with.

The problem is I still need the regular list of annotations (in addition to the annotationsToKeep). I can still move it to a separate function though.

Also I think I resolved this by not using allDiscoveredAnnotations so now excluded args should get handled correctly.

cmnbroad · 2022-10-04T21:27:02Z

.../java/org/broadinstitute/hellbender/cmdline/GATKPlugin/GATKAnnotationArgumentCollection.java

@@ -37,4 +37,7 @@ public abstract class GATKAnnotationArgumentCollection implements Serializable {
     * Returns {@code true} if all annotations are enabled; {@code false} otherwise.
     */
    public abstract boolean getEnableAllAnnotations();
+
+
+    public abstract List<String> getKeepSpecifiedCombinedAnnotationNames();


Also this can be removed.

cmnbroad · 2022-10-04T21:28:12Z

...in/java/org/broadinstitute/hellbender/cmdline/GATKPlugin/GATKAnnotationPluginDescriptor.java

@@ -497,4 +496,12 @@ public Class<?> getClassForPluginHelp(final String pluginName) {
        return allDiscoveredAnnotations.containsKey(pluginName) ? allDiscoveredAnnotations.get(pluginName).getClass() : null;
    }

+    public Map<String,Annotation> getAllDiscoveredAnnotations() {


If we keep this method, I'd suggest:

moving it so its adjacent to getResolvedInstances

renaming it to getDiscoveredInstances

adding a javadoc comment saying that it returns every annotation that is discovered anywhere on the classpath, and that it should only be used when getResolvedInstances isn't sufficient

meganshand

Thank you @cmnbroad! I knew it wasn't right, but I was missing the key step of declaring the Argument Collection in GenotpeGVCFs without labeling it an ArgumentCollection.

Hopefully it's cleaner now and should be ready for re-review.

meganshand · 2022-10-06T20:06:47Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+        final Map<String, Annotation> allDiscoveredAnnotations = pluginDescriptor.getAllDiscoveredAnnotations();
+        final Set<Annotation> annotationsToKeep = annotationStringsToKeep.stream().map(allDiscoveredAnnotations::get).collect(Collectors.toSet());


The problem is I still need the regular list of annotations (in addition to the annotationsToKeep). I can still move it to a separate function though.

Also I think I resolved this by not using allDiscoveredAnnotations so now excluded args should get handled correctly.

cmnbroad

@meganshand The argument stuff looks much better now, since everything specific to GenotypeGVCFs is private to it now, but see my comments about the list reconciliation.

...in/java/org/broadinstitute/hellbender/cmdline/GATKPlugin/GATKAnnotationPluginDescriptor.java

cmnbroad · 2022-10-11T20:30:14Z

...in/java/org/broadinstitute/hellbender/cmdline/GATKPlugin/GATKAnnotationPluginDescriptor.java

+     * Returns a map of the String to Annotations only in the resolved instances. Should only be used if
+     * getResolvedInstances() is not sufficient.


I think the "Should be used...." warning can be removed. That comment was intended for a previous revision where you were exposing allDiscoveredAnnotations.

cmnbroad · 2022-10-11T20:43:32Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/GenotypeGVCFs.java

+        final GATKAnnotationPluginDescriptor pluginDescriptor = getCommandLineParser().getPluginDescriptor(GATKAnnotationPluginDescriptor.class);
+        final List<String> annotationStringsToKeep = genotypeGVCFsAnnotationArgs.getKeepSpecifiedCombinedAnnotationNames();
+        final Map<String, Annotation> resolvedInstancesMap = pluginDescriptor.getResolvedInstancesMap();
+        return annotationStringsToKeep.stream().map(resolvedInstancesMap::get).collect(Collectors.toSet());


I think you need to be a bit more defensive in how you reconcile the "resolved" entries returned by getResolvedInstancesMap() with the entries in annotationStringsToKeep. You're not guaranteed that resolvedInstancesMap will contain an entry for every annotation the user specified in annotationStringsToKeep, so you need to handle the case where resolvedInstancesMap::get returns null here.

For instance, I don't think there is anything that will detect that the user made up some non-existent annotation name and specified it as a keeper, since they're just strings up until this point. Also, it's possible for the user to have both excluded an annotation AND specified it as a keeper (in which case it won't be in the resolved list). This code needs to detect and reject those cases so it doesn't wind up handing the annotator engine null annotations, preferably with some tests to ensure they're rejected properly.

You probably won't be able to tell exactly WHY a null is returned here, but I think rejecting that case with an error message saying something like, the annotation zzz either does not exist or was excluded. It's a bit of a pain - a lot of the logic in the plugin descriptor is to do exactly this kind of reconciliation and error reporting, but since the keep list is specific to GenotypeGVCFs, reconciling part of it has to be done here.

I moved the "missing annotation" check here and added a test.

…/GATKAnnotationPluginDescriptor.java Co-authored-by: Chris Norman <[email protected]>

meganshand · 2022-10-14T15:15:16Z

Thanks @cmnbroad, back to you!

cmnbroad

@meganshand I mostly focused on the arg part (which looks good now - phew!) on the theory that Sam reviewed the rest of this. I added one more nit/comment, up to you if you want to use it or not. Otherwise LGTM.

cmnbroad · 2022-10-18T18:34:26Z

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

        Utils.nonNull(featureInputs, "comparisonFeatureInputs is null");
        infoAnnotations = new ArrayList<>();
        genotypeAnnotations = new ArrayList<>();
        jumboInfoAnnotations = new ArrayList<>();
        jumboGenotypeAnnotations = new ArrayList<>();
+        final List<String> variantAnnotationKeys = new ArrayList<>();
+        final List<String> rawVariantAnnotationKeysToKeep = new ArrayList<>();


One other minor nit: why introduce this intermediate alias ? To me having the additional alias with a slightly different name just obfuscates whats going on - I'd suggest just allocating and using the rawAnnotationsToKeep instance variable directly, since this winds up being assigned there anyway.

Yup, good catch. I fixed this and removed variantAnnotationKeys since it wasn't being used anymore (after moving the error message to GenotypeGVCFs)

meganshand · 2022-10-18T19:19:54Z

@cmnbroad I think Sam has reviewed the rest of this PR. Thank you for helping me get the argument part working! Assuming tests pass this is ready for a final review.

samuelklee · 2022-10-18T20:44:03Z

@meganshand you've got a prior approval from me, but let me know if you think I should re-review. Thanks @cmnbroad for helping out with the argument code and reviewing!

cmnbroad

LGTM.

adding argument to GenotypeGVCFs to keep only RAW_GT_COUNT (not all r…

9ec8ca4

…aw annotations)

meganshand requested a review from samuelklee August 22, 2022 14:55

making the argument more general

4a8b3cf

making arg optional

522cd29

samuelklee approved these changes Sep 8, 2022

View reviewed changes

meganshand added 2 commits September 21, 2022 14:02

WIP

470c858

WIP - adding annotation list as argument

bf9fffc

cmnbroad and others added 3 commits September 27, 2022 11:42

Integrate GenotypeGVCFsAnnotationArgumentCollection.

263d2d8

trying to fix annotation argument

7462183

addressing comments

aaad296

fixing ReblockGvcf tests

65e4309

cmnbroad requested changes Oct 4, 2022

View reviewed changes

addressing argument collection comments

27b3994

meganshand commented Oct 6, 2022

View reviewed changes

cmnbroad requested changes Oct 11, 2022

View reviewed changes

meganshand and others added 2 commits October 13, 2022 14:04

Update src/main/java/org/broadinstitute/hellbender/cmdline/GATKPlugin…

fdccc7b

…/GATKAnnotationPluginDescriptor.java Co-authored-by: Chris Norman <[email protected]>

addressing comments

8d314f6

cmnbroad reviewed Oct 18, 2022

View reviewed changes

cleaning up

f7b8aa9

cmnbroad approved these changes Oct 24, 2022

View reviewed changes

meganshand merged commit 90bf668 into master Oct 24, 2022

meganshand deleted the ms_raw_annots branch October 24, 2022 20:20

		@@ -942,6 +941,7 @@ public void testRawGtCountAnnotation() {
		Assert.assertEquals(rawGtCount.get(0), ".");

		final Map<String, Annotation> allDiscoveredAnnotations = pluginDescriptor.getAllDiscoveredAnnotations();
		final Set<Annotation> annotationsToKeep = annotationStringsToKeep.stream().map(allDiscoveredAnnotations::get).collect(Collectors.toSet());

		* Returns a map of the String to Annotations only in the resolved instances. Should only be used if
		* getResolvedInstances() is not sufficient.

Adding argument to GenotypeGVCFs to keep only RAW_GT_COUNT #7996

Adding argument to GenotypeGVCFs to keep only RAW_GT_COUNT #7996

Conversation

meganshand commented Aug 22, 2022

codecov bot commented Aug 22, 2022 • edited Loading

Codecov Report

samuelklee commented Aug 22, 2022

gatk-bot commented Aug 24, 2022 • edited Loading

meganshand commented Aug 25, 2022

samuelklee commented Aug 29, 2022

samuelklee 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

samuelklee Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelklee Sep 8, 2022 • edited Loading

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

samuelklee Sep 8, 2022 • edited Loading

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

gatk-bot commented Sep 26, 2022 • edited Loading

gatk-bot commented Sep 30, 2022 • edited Loading

meganshand commented Sep 30, 2022

cmnbroad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnbroad Oct 4, 2022 • edited Loading

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

meganshand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnbroad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnbroad Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meganshand commented Oct 14, 2022

cmnbroad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meganshand commented Oct 18, 2022

samuelklee commented Oct 18, 2022

cmnbroad left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 22, 2022 •

edited

Loading

gatk-bot commented Aug 24, 2022 •

edited

Loading

samuelklee Sep 30, 2022 •

edited

Loading

samuelklee Sep 8, 2022 •

edited

Loading

samuelklee Sep 8, 2022 •

edited

Loading

gatk-bot commented Sep 26, 2022 •

edited

Loading

gatk-bot commented Sep 30, 2022 •

edited

Loading

cmnbroad Oct 4, 2022 •

edited

Loading

cmnbroad Oct 11, 2022 •

edited

Loading