sarek icon indicating copy to clipboard operation
sarek copied to clipboard

Published 20 hours ago • verified publisher icon nf-core

Update CNVkit

Open lescai opened this issue 3 months ago • 5 comments

PR checklist

Overall description

This PR adds the following improvements to the CNVkit subworkflow(s):

  • updates the CNVkit modules, as updated in nf-core/modules
  • with the revised cnvkit/batch module, makes handling params.cnvkit_reference uniform across the three subworkflows (germline, tumour only, tumour-normal)
  • adds a specific cnvkit/call module to perform calling from cns files
  • adds the cnvkit/export module, to export called CNVs into VCF, thus uniforming Sarek variant results
  • adds the recommended settings to cnvkit/call config for germline workflows

Checklist

  • [ ] The code has been tested on real germline samples, both from FASTQ and BAM files: the automated tests might still need a bit of work
  • [ ] If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • [ ] If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repository.
  • [ ] Make sure your code lints (nf-core lint).
  • [ ] Ensure the test suite passes (nf-test test tests/ --verbose --profile +docker).
  • [ ] Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • [ ] Usage Documentation in docs/usage.md is updated.
  • [ ] Output Documentation in docs/output.md is updated.
  • [ ] CHANGELOG.md is updated.
  • [ ] README.md is updated (including new tool citations and authors/contributors).

lescai avatar May 06 '24 09:05 lescai

nf-core lint overall result: Passed :white_check_mark: :warning:

Posted for pipeline commit bb0ec8d

+| ✅ 197 tests passed       |+
#| ❔  15 tests were ignored |#
!| ❗   3 tests had warnings |!

:heavy_exclamation_mark: Test warnings:

  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!

:grey_question: Tests ignored:

  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: conf/modules.config
  • files_exist - File is ignored: lib/WorkflowMain.groovy
  • files_exist - File is ignored: lib/NfcoreTemplate.groovy
  • files_exist - File is ignored: lib/WorkflowSarek.groovy
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md
  • files_unchanged - File ignored due to lint config: assets/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_dark.png
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • actions_ci - actions_ci
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/sarek/sarek/.github/workflows/awstest.yml
  • template_strings - template_strings
  • modules_config - modules_config

:white_check_mark: Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-05-11 13:39:52

github-actions[bot] avatar May 06 '24 09:05 github-actions[bot]

proposals for docs and changelog updates just submitted. feel free to modify :)

lescai avatar May 06 '24 21:05 lescai

Looks very good 👏👏 What about CI-tests covering this new functionality?

asp8200 avatar May 11 '24 07:05 asp8200

I've added 2 emits for the raw and exported CNV calls. However, at the moment there's no merging of SNP/INDEL variant calls and CNV calls into the variant calling subworkflow(s). This is a feature requested by users, to my understanding. This emit will ensure being able to add this in the future, but unless some additional work is done on the bam_variant_calling_* subworkflows, the emitted channels are not used at the moment. This is beyond the scope of this PR though :)

lescai avatar May 11 '24 13:05 lescai

Thanks for the updates 🙌

Concerning CI-tests for this new functionality, I see we already have this pytest of the germline subworkflow of cnvkit, and it runs CNVKIT_EXPORT and generates results/variant_calling/cnvkit/sample1/sample1.cnvcall.vcf.

Currently that pytest doesn't check for the existence of sample1.cnvcall.vcf - perhaps it should?

EDIT: Perhaps also check for the existence of results/variant_calling/cnvkit/sample1/test.paired_end.recalibrated.sorted.germline.call.cns which seems to be generated by CNVKIT_CALL in that pytest?

asp8200 avatar May 12 '24 09:05 asp8200