VS-1395 Have create vat from vds optionally take sites only vcf #8851
Conversation
…terIzeCreateVATFromVDS
…terIzeCreateVATFromVDS
…ve_CreateVATFromVDS_OptionallyTakeSitesOnlyVCF
| @@ -1307,3 +1309,47 @@ task PopulateFilterSetInfo { | |||
| File monitoring_log = "monitoring.log" | |||
| } | |||
| } | |||
|
|
|||
| # TODO - should put in a do not overwrite that file logic | |||
There was a problem hiding this comment.
is this going to be covered by this PR/ticket or a subsequent one?
There was a problem hiding this comment.
I'll make a ticket for that.
There was a problem hiding this comment.
Actually, I ended up doing that work in this PR>
Co-authored-by: Bec Asch <[email protected]>
Co-authored-by: Bec Asch <[email protected]>
| hail_generate_sites_only_script_path = select_first([hail_generate_sites_only_script_path]), | ||
| ancestry_file_path = MakeSubpopulationFilesAndReadSchemaFiles.ancestry_file_path, | ||
| workspace_bucket = GetToolVersions.workspace_bucket, | ||
| region = "us-central1", |
There was a problem hiding this comment.
I realize this is preexisting but IMHO the region should be out in a declaration rather than hardcoded
There was a problem hiding this comment.
and definitely no need to consult git blame on the baseline code
| input_vcf = GenerateSitesOnlyVcf.sites_only_vcf, | ||
| gatk_docker = effective_gatk_docker, | ||
| input_file = select_first([sites_only_vcf_index, IndexVcf.output_vcf_index]), | ||
| output_gcs_dir = output_path + "sites_only_vcf", |
There was a problem hiding this comment.
are we sure there will be a trailing slash on output_path?
There was a problem hiding this comment.
I added code to validate that it does end with a trailing slash.
…ve_CreateVATFromVDS_OptionallyTakeSitesOnlyVCF
| if (!defined(split_intervals_scatter_count)) { | ||
| call Utils.GetBQTableLastModifiedDatetime as SampleDateTime { | ||
| String output_path_without_a_trailing_slash = sub(output_path, "/$", "") | ||
| if (output_path == output_path_without_a_trailing_slash) { |
|
|
||
| call Utils.GetNumSamplesLoaded { | ||
| if (!defined(sites_only_vcf) && ((!defined(vds_path) || !defined(hail_generate_sites_only_script_path)))) { |
There was a problem hiding this comment.
Maybe treat the two cases separately? If sites_only_vcf then those other two params really shouldn't be?
There was a problem hiding this comment.
Yeah. I wondered about it. I kind of figured there's no harm if a user leaves an unneeded parameter defined, but it could cause confusion.
…ve_CreateVATFromVDS_OptionallyTakeSitesOnlyVCF
gbggrant
deleted the
gg_VS-1395_Have_CreateVATFromVDS_OptionallyTakeSitesOnlyVCF
branch
* Have GvsCreateVATfromVDS.wdl take sites-only-vcf as an optional input. * Added logic to allow/disallow CopyFile to overwrite.
Modify GvsCreateVATFromVDS to take as an optional input the
sites_only_vcf- if provided, the code bypasses the logic to create it from VDS.This PR also modifies IndexVcf and SelectVariants to use localization_optional for their inputs.
Updated Passing Integration test here
Rerun of GvsCreateVATFromVDS using a passed in sites_only VCF here.
Rerun of GvsCreateVATFromVDS NOT using a passed in sites_only VCF here.