-
Notifications
You must be signed in to change notification settings - Fork 14
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
Move variant discovery to its own subcommand #234
Conversation
I have (successfully) run the tip of this branch on multiple samples. |
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 two minor comments, very nice PR, I think this was an essential refactoring, thanks for taking the time to do this
As a way of validating this is working - in tandem with mbhall88/head_to_head_pipeline#54 - here is a picture of the precision and recall for the version of pandora in this PR fixed_prgs is the data from this PR RecallPrecisionNote: fixed_prgs is unfiltered here |
I found myself needing to expose more and more de novo parameters at the CLI and realised it was all cluttering
map
. There could also a slightly misleading perception that using--discover --genotype
inmap
would genotype with the discovered variants (which it doesn't). Anyway, I forsee - when we have self-container de novo variant integration - the need to have this subcommand output a new PRG with variants added, or even another subcommandaugment
.Added
pandora discover
-d,--merge
to specify the distance at CLIInterval::is_close
method--min-dbg-dp
to expose minimum adbundance in GATB graph-l
and-L
for min/max candidate region length-P,--pad
to specify candidate region paddingChanged
-g,--genome-size
to take a string such as4.4m
[closes Allow passing genome size string #233]Removed
--coverages
option frommap
as it wasn't being used