-
Notifications
You must be signed in to change notification settings - Fork 597
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 vqsr cutoffs to GvsExtractCallset wdl; clean up dockstore yml #7209
Conversation
@@ -17,6 +17,13 @@ workflow GvsExtractCallset { | |||
String? fq_filter_set_site_table | |||
String? fq_filter_set_tranches_table | |||
String? filter_set_name | |||
|
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 we make the corresponding changes above to default these table names given a "default_dataset" etc? We can also make them non-optional, though we would need to check that works with ExtractCohort (ie if I specify those table names, but no filter set... I mean "don't filter"... we might explode right now in that scenario)
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.
yeah, I'll give it a shot, we should think about what the decision-making input will be that decides whether to filter or not. right now it's whether --filter-set-info-table
is input to the tool - maybe we should change that to be whether a filter set name is input? then we could have defaults for the table names like you suggest
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.
@kcibul how does this look to you now? i tested that it works with and without filtering in Terra (i.e. providing filter-table-name, and setting do_not_filter_override to true, respectively)
c0523e8
to
e61823e
Compare
…meres.interval_list
specops issue #287
tested with defined truth sensitivities here: https://app.terra.bio/#workspaces/broad-dsp-spec-ops-fc/gvs_testing_no_sa/job_history/39f8c4ea-8e21-46d4-a6ba-8e570ce75f8b