Repository navigation
Merge BAM_SOMATIC_VARIANT_CALLING_GATK subworkflows - #13335
beatrizsavinhas wants to merge 34 commits into
Conversation
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
| .map { meta_tumor, bam_tumor, bai_tumor, meta_normal, bam_normal, bai_normal -> | ||
| [[id: "${meta_tumor.id}_${meta_normal.id}" ], [bam_tumor, bam_normal], [bai_tumor, bai_normal]] | ||
| } | ||
| .combine(ch_interval_file) |
There was a problem hiding this comment.
In the original code, the interval file was not used here, but looking at the GATK4_MUTECT2 it is possible to provide it too. This is why the md5sums of the output files also differ from the original test snapshot.
Please, let me know if adding the interval file is not relevant here for some reason I overlooked!
| .combine(ch_interval_file) | ||
|
|
||
| // Ensure that the panel of normals is not used for tumor-normal analysis, even if provided | ||
| ch_panel_of_normals = [] |
There was a problem hiding this comment.
This might be better handled at the module level.
There was a problem hiding this comment.
Do you suggest updating the module code instead? Currently there is no logic to cover this in the GATK4_MUTECT2 module.
There was a problem hiding this comment.
Yes, I think that would be a much better approach if possible. Updating the module logic makes the workflow cleaner and easier to maintain.
There was a problem hiding this comment.
OK, fair! I will address that issue in a separate PR then!
There was a problem hiding this comment.
I've looked into the module and this is not straightforward as there is no logic to determine if a matched normal sample is provided based on the inputs -
.The functionality above has now been removed from this PR so it is up to the user to provide the correct inputs (clear instructions were added to the
meta.yml file). Since this was already the case before this PR, and substantial changes to the module would be required to add the check there, perhaps we can settle for leaving the proper usage of the subworkflow/module up to the user.
|
I have now implemented a different approach restoring the joint input channel and addressing #13335 (comment) which also removes the dependency on Could you take another look at the code @imsarath? |
| ch_getpileupsummaries_in = ch_input | ||
| .flatMap { meta, tumor_input, tumor_input_index, normal_input, normal_input_index -> | ||
| def result = [ | ||
| [[id: "${meta.id}_tumor", original_meta: meta], // Add unique key so output files receive differing names |
There was a problem hiding this comment.
This logic and revert in line 92 could be bypassed by setting ext.prefix for GATK4_GETPILEUPSUMMARIES as was done previously, but the would mean that the subworkflow would not work at all without that configuration. I thought adding the logic to the subworkflow itself would be more explicit and provide a better user experience.
2 participants
PR checklist
Create
BAM_SOMATIC_VARIANT_CALLING_GATKsubworkflow merging the logic forBAM_TUMOR_NORMAL_SOMATIC_VARIANT_CALLING_GATKandBAM_TUMOR_ONLY_SOMATIC_VARIANT_CALLING_GATK. A single subworkflow capable of performing both analysis types reduces code duplication in pipelines that can run both tumor only and tumor-normal analyses.topic: versions- See version_topicslabelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile condanf-core subworkflows test <SUBWORKFLOW> --profile dockernf-core subworkflows test <SUBWORKFLOW> --profile singularitynf-core subworkflows test <SUBWORKFLOW> --profile conda