-
Notifications
You must be signed in to change notification settings - Fork 596
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
Created VariantAnnotationArgumentCollection and StandardM2Annotation classes #3621
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
...institute/hellbender/cmdline/argumentcollections/VariantAnnotationArgumentCollection.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package org.broadinstitute.hellbender.cmdline.argumentcollections; | ||
|
||
import org.broadinstitute.barclay.argparser.Advanced; | ||
import org.broadinstitute.barclay.argparser.Argument; | ||
import org.broadinstitute.hellbender.cmdline.StandardArgumentDefinitions; | ||
import org.broadinstitute.hellbender.utils.Utils; | ||
|
||
import java.io.Serializable; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
/** | ||
* Arguments for requesting VariantContext annotations to be processed by {@link org.broadinstitute.hellbender.tools.walkers.annotator.VariantAnnotatorEngine} | ||
* for tools that process variants objects. | ||
*/ | ||
public class VariantAnnotationArgumentCollection implements Serializable { | ||
private static final long serialVersionUID = 1L; | ||
|
||
/** | ||
* Argument collection constructor which defines tool default annotation arguments when they are not overridden by the user | ||
* | ||
* @param defaultGroups List annotation group names to be used by default | ||
* @param defaultAnnotations List of annotation class names to be used by default | ||
* @param defaultAnnotationsToExclude List of annotation class names to exclude by default. These override the default annotations and annotation groups. | ||
*/ | ||
public VariantAnnotationArgumentCollection(List<String> defaultGroups, List<String> defaultAnnotations, List<String> defaultAnnotationsToExclude) { | ||
Utils.nonNull(defaultGroups); | ||
Utils.nonNull(defaultAnnotations); | ||
Utils.nonNull(defaultAnnotationsToExclude); | ||
|
||
annotationGroupsToUse = new ArrayList<>(defaultGroups); | ||
annotationsToUse = new ArrayList<>(defaultAnnotations); | ||
annotationsToExclude = new ArrayList<>(defaultAnnotationsToExclude); | ||
} | ||
|
||
/** | ||
* Which annotations to include in variant calls in the output. These supplement annotations provided by annotation groups. | ||
*/ | ||
@Argument(fullName=StandardArgumentDefinitions.ANNOTATION_LONG_NAME, shortName=StandardArgumentDefinitions.ANNOTATION_SHORT_NAME, doc="One or more specific annotations to add to variant calls", optional=true) | ||
public List<String> annotationsToUse = new ArrayList<>(); | ||
|
||
/** | ||
* Which annotations to exclude from output in the variant calls. Note that this argument has higher priority than the | ||
* -A or -G arguments, so these annotations will be excluded even if they are explicitly included with the other | ||
* options. | ||
*/ | ||
@Argument(fullName=StandardArgumentDefinitions.ANNOTATIONS_TO_EXCLUDE_LONG_NAME, shortName=StandardArgumentDefinitions.ANNOTATIONS_TO_EXCLUDE_SHORT_NAME, doc="One or more specific annotations to exclude from variant calls", optional=true) | ||
public List<String> annotationsToExclude = new ArrayList<>(); | ||
|
||
/** | ||
* Which groups of annotations to add to the output variant calls. | ||
* Any requirements that are not met (e.g. failing to provide a pedigree file for a pedigree-based annotation) may cause the run to fail. | ||
*/ | ||
@Argument(fullName= StandardArgumentDefinitions.ANNOTATION_GROUP_LONG_NAME, shortName=StandardArgumentDefinitions.ANNOTATION_GROUP_SHORT_NAME, doc="One or more groups of annotations to apply to variant calls", optional=true) | ||
public List<String> annotationGroupsToUse = new ArrayList<>(); | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 1 addition & 4 deletions
5
src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/BaseQuality.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 1 addition & 4 deletions
5
src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/FragmentLength.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 1 addition & 6 deletions
7
src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/MappingQuality.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 2 additions & 2 deletions
4
.../annotator/StandardSomaticAnnotation.java → ...s/annotator/StandardMutectAnnotation.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
package org.broadinstitute.hellbender.tools.walkers.annotator; | ||
|
||
/** | ||
* This is a marker interface used to indicate which annotations are "Standard" for MuTect only. | ||
* This is a marker interface used to indicate which annotations are "Standard" for Mutect2 only. | ||
*/ | ||
public interface StandardSomaticAnnotation { | ||
public interface StandardMutectAnnotation extends Annotation { | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please, consider to make this an abstract class to being able to hide some parameters from the command line. Then, a
DefaultGATKVariantAnnotationArgumentCollection
should be used in GATK with this implementation.If the variant anotation engine is converted into a plugin, that will be useful for downstream projects: I was actually working on my own plugin, but I prefer to work with standards from the GATK framework, and if there are downstream projects that want to annotate variants with their own annotations, this will be required for example if they do not need groups to annotate together.
As an example, that will be similar to what I did for the
GATKReadFilterPluginDescriptor
in #2355. As an API user, I think that it is good to have all the arguments for the plugins as abstract classes to being able to hide some from the CLI...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.
@magicDGS This would be a useful feature but it sounds like the benefit is mostly hidden behind pending work on converting annotations into a plugin, which we have planned for the 4.0 release. This change would probably be incorporated as part of that branch.
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.
I thought that this could be addressed here for using the
VariantAnnotationArgumentCollection
with theVariantAnnotator
too, even if the plugin is not there. I cannot hide some parameters with the current implementation, to being able to pass the argument collection to the annotator...Anyway, I can submit a patch if there is no planned timeplan for the plugin, to being able to use something similar in my code...
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.
Maybe it would be easier to just add a generic/universal way to hide arguments in barclay, instead of having to complicate all of our argument collections with added abstract classes?
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.
I think that the main reason of having argument collections is for tweak the arguments for
make optional/required some arguments (such as
ReferenceInputArgumentCollection
) or have the ability to tweak some arguments to be hidden/removed from the user.I believe that with the current system for parsing arguments and argument collections is going to be complicated to hide arguments. In addition, sometimes the requirement isn't hide the argument but provide a tool-specific documentation (e.g., the
ReadFilter
plugin specifies that are filters before the analysis; in some cases, I have tools which perform the filtering after processing, and it is important for the user to know that difference).I think that having interfaces for a framework is the best way to encourage users with specific needs to use it, unless you really want to forbid the usage/tweak of some components. I see several use cases for having the abstract argument collections....