Skip to content

sniffles/genotypevcf: new module - #13105

Closed
aksenia wants to merge 3 commits into
nf-core:masterfrom
aksenia:sniffles-genotypevcf
Closed

aksenia wants to merge 3 commits into
nf-core:masterfrom
aksenia:sniffles-genotypevcf

Conversation

@aksenia

@aksenia aksenia commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Adds sniffles/genotypevcf, which determines the genotypes of a given set of structural variants in a long-read alignment with Sniffles forced calling (--genotype-vcf). It lives next to the existing sniffles module because it takes a different input (a VCF of known SVs) and produces genotypes of those records rather than de novo calls.

  • A gzipped sites VCF is decompressed first, since Sniffles reads the file as plain text (Sniffles 2.8.1 bug with handling vcf.gz files fritzsedlazeck/Sniffles#613).
  • Outputs the genotyped vcf.gz with its tbi; version broadcast via topic: versions.
  • Tests use a small HG002 ONT subset and a matching known-SV VCF, added in Add HG002 ONT SV genotyping test data test-datasets#2302. No existing long-read BAM in test-datasets has a matching known-SV VCF at a depth where heterozygous and homozygous genotypes can be called (the SAVANA COLO829 subset is ~1-2x with no SV set; test.sorted.bam has contigs < 1 Mb, which Sniffles skips). The data is merged. The tests (vcf.gz, plain vcf, stub) pass with the docker profile on amd64, and the snapshot comes from that run.

Generated by Claude

PR checklist

Closes #13106

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • If necessary, include test data in your PR. (test data PR: Add HG002 ONT SV genotyping test data test-datasets#2302)
  • Remove all TODO statements.
  • Broadcast software version numbers to topic: versions - See version_topics
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

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.

@aksenia aksenia self-assigned this Oct 8, 2026
@aksenia
aksenia marked this pull request as ready for review October 8, 2026 11:41
@awgymer

awgymer commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Ok, so a couple of fundamental issues here:

  1. This attempts to create a <tool>/<subtool> module when <tool> already exists. This is something we really do not like to do. It has happened previously (with deepvariant, possibly others) and what actually needs to happen is that the <tool> module is deprecated and a new <tool>/<subtool> module created implementing the same logic.

  2. Despite your claim that this needs to be a new module because it takes a different input, it's not strictly true AFAICT from the README.

D. Genotyping a known set of SVs (Force Calling)

Example command, to determine the genotype of each SV in input_known_svs.vcf for sample.bam and write the re-genotyped SVs to output_genotypes.vcf:

sniffles --input sample.bam --genotype-vcf input_known_svs.vcf --vcf output_genotypes.vcf

From this you can see it still takes a bam via --input, it also still uses --vcf for its output. The only difference is it takes an additional --genotype-vcf. This could be incorporated into the existing module via an additional optional input field in the tuple and a piece of logic to populate the arg:

input:
tuple val(meta), path(input), path(index), path(genotype_vcf)
...

script:
...
def genotype = genotype_vcf ? "--genotype-vcf ${genotype_vcf}" : ""

"""
sniffles \\
    --input $input \\
    $reference \\
    -t $task.cpus \\
    $tandem_repeats \\
    $genotype \\
    $vcf \\
    $snf \\
    $args
"""

@aksenia
aksenia requested review from mashehu and removed request for mashehu October 9, 2026 06:32
@aksenia

aksenia commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Hi @awgymer. I was weighing between the two options and went for new module as this was how I read the nf-core guidelines - that there should be one task / functionality per module, otherwise it should be split - this is what I attempted. Folding it into existing will be much easier, I will do it in a new PR and close this one. Thanks for review!

@aksenia

aksenia commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing - there will be a new PR.

@aksenia aksenia closed this Oct 9, 2026
@aksenia
aksenia deleted the sniffles-genotypevcf branch October 9, 2026 06:49
@awgymer

awgymer commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Generally if a single command (here sniffles) has some variable behaviours based on optional inputs it is acceptable to still have one module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sniffles: add forced calling (--genotype-vcf)

2 participants