-
Notifications
You must be signed in to change notification settings - Fork 586
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
add microbial mode #6694
add microbial mode #6694
Conversation
1ee14e0
to
e8c4972
Compare
be324f5
to
7dc51cd
Compare
e8c4972
to
fa3240a
Compare
4c6f006
to
ff60e32
Compare
ff60e32
to
9690dac
Compare
9690dac
to
4acfd7a
Compare
...itute/hellbender/tools/walkers/haplotypecaller/readthreading/AbstractReadThreadingGraph.java
Outdated
Show resolved
Hide resolved
@ahaessly I've asked @jamesemery to review, as this touches some of his code. In the mean time, though, could you please add at least one good integration test for the new |
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.
A bunch of minor comments. It would generally be nice for there to be more comments and method documentation in the tool you added. More importantly I think we really need to have at least a simple integration test for that tool that at the very least asserts it does something reasonable on some fasta (it doesn't have to be a real file you can just use some dummy fasta files).
...itute/hellbender/tools/walkers/haplotypecaller/readthreading/ReadThreadingGraphUnitTest.java
Show resolved
Hide resolved
...itute/hellbender/tools/walkers/haplotypecaller/readthreading/AbstractReadThreadingGraph.java
Outdated
Show resolved
Hide resolved
@@ -43,6 +43,13 @@ | |||
@Argument(fullName = M2ArgumentCollection.MITOCHONDRIA_MODE_LONG_NAME, optional = true, doc = "Set filters to mitochondrial defaults") | |||
public boolean mitochondria = false; | |||
|
|||
/** | |||
* Mitochondria mode excludes the filters {@link ClusteredEventsFilter}, {@link MultiallelicFilter}, |
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.
Can you update this list to be comprehensive to all the changes?
Also were you not going to make this argument affect the graph code at all? I thought bacterial mode would want to use the new dangling end pruning code and that doesn't seem to be linked to microbial mode at all.
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.
This also doesn't refrence the median mapping wquality change.
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.
right now that flag is specified in the wdl for microbial mutect. We may decide to add it to the code when using this mode in a later PR
...rg/broadinstitute/hellbender/tools/walkers/mutect/filtering/M2FiltersArgumentCollection.java
Outdated
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Outdated
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Outdated
Show resolved
Hide resolved
...rg/broadinstitute/hellbender/tools/walkers/mutect/filtering/M2FiltersArgumentCollection.java
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Show resolved
Hide resolved
f3737bb
to
3f39bee
Compare
author Andrea Haessly <[email protected]> 1582141271 -0500 committer Andrea Haessly <[email protected]> 1611852899 -0500 parent 6d43103 author Andrea Haessly <[email protected]> 1582141271 -0500 committer Andrea Haessly <[email protected]> 1611852778 -0500 initial impl tested shift and chain make offset optional fix format of chain file make offset optional fix format of chain file add interval output
3f39bee
to
a4f8290
Compare
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.
Just a few very minor comments to address/answer then i think this can be merged
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Outdated
Show resolved
Hide resolved
src/test/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFastaTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFastaTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/walkers/fasta/ShiftFasta.java
Show resolved
Hide resolved
.../org/broadinstitute/hellbender/tools/walkers/haplotypecaller/graphs/AdaptiveChainPruner.java
Show resolved
Hide resolved
…nce coverage sites\nAdd large files to resources
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.
This looks good to me. Feel free to merge when tests pass!
No description provided.