5787 new module whatshap/stats #9490

eliottBo · 2025-12-02T09:50:00Z

Add module whatshap/stats.

PR checklist

Closes #5787

…tput in a txt file

…modules into 5787-new-module-whatshap-stats

modules/nf-core/whatshap/stats/main.nf

nschan · 2025-12-02T14:31:31Z

This looks overall fine. My concern is that whatshap stats has a bunch of arguments that are not clearly documented on readthedocs (https://github.com/whatshap/whatshap/blob/main/whatshap/cli/stats.py):

    add("--gtf", metavar="FILE", help="Write phased blocks as GTF with each block represented as a "
        "'gene'. If blocks are interleaved or nested, they are split into multiple 'exons'.")
    add("--block-list", metavar="FILE", help="Write list of all blocks to FILE (one block per "
        "line). Nested/interleaved blocks are not split.")
    add("--sample", metavar="SAMPLE", help="Name of the sample "
        "to process. If not given, use first sample found in VCF.")
    add("--chr-lengths", metavar="FILE",
        help="Override chromosome lengths in VCF with those from FILE (one line per chromosome, "
        "tab separated '<chr> <length>'). Lengths are used to compute NG50 values.")
    add("--tsv", metavar="FILE", help="Write statistics in tab-separated value format to FILE")
    add("--only-snvs", default=False, action="store_true", help="Only process SNVs "
        "and ignore all other variants.")
    add("--chromosome", dest="chromosomes", metavar="CHROMOSOME", default=[], action="append",
        help="Name of chromosome(s) to process. If not given, all chromosomes in the "
        "input VCF are considered. Can be used multiple times and accepts a comma-separated list. ")
    add("vcf", metavar="VCF", help="Phased VCF file")

And it looks like the current implementation uses --tsv, but I think it would be nice to support maybe an input switch for format, to toggle between gtf, block-list and tsv? I think the other arguments can come via $args but additional output formats would not be captured as it is now.

…modules into 5787-new-module-whatshap-stats

eliottBo · 2025-12-03T15:27:34Z

This looks overall fine. My concern is that whatshap stats has a bunch of arguments that are not clearly documented on readthedocs (https://github.com/whatshap/whatshap/blob/main/whatshap/cli/stats.py):

    add("--gtf", metavar="FILE", help="Write phased blocks as GTF with each block represented as a "
        "'gene'. If blocks are interleaved or nested, they are split into multiple 'exons'.")
    add("--block-list", metavar="FILE", help="Write list of all blocks to FILE (one block per "
        "line). Nested/interleaved blocks are not split.")
    add("--sample", metavar="SAMPLE", help="Name of the sample "
        "to process. If not given, use first sample found in VCF.")
    add("--chr-lengths", metavar="FILE",
        help="Override chromosome lengths in VCF with those from FILE (one line per chromosome, "
        "tab separated '<chr> <length>'). Lengths are used to compute NG50 values.")
    add("--tsv", metavar="FILE", help="Write statistics in tab-separated value format to FILE")
    add("--only-snvs", default=False, action="store_true", help="Only process SNVs "
        "and ignore all other variants.")
    add("--chromosome", dest="chromosomes", metavar="CHROMOSOME", default=[], action="append",
        help="Name of chromosome(s) to process. If not given, all chromosomes in the "
        "input VCF are considered. Can be used multiple times and accepts a comma-separated list. ")
    add("vcf", metavar="VCF", help="Phased VCF file")

And it looks like the current implementation uses --tsv, but I think it would be nice to support maybe an input switch for format, to toggle between gtf, block-list and tsv? I think the other arguments can come via $args but additional output formats would not be captured as it is now.

Thanks for your review @nschan , I now updates my PR for the user to be able to choose which output they want to make it more flexible.

nschan · 2025-12-03T16:14:10Z

Thanks, that looks very good to me. I have one additional question, probably because I have no experience with it: independent of any --gtf or --tsv setting, will it always print to stdout, i.e. can you capture the stats with > ${prefix}.txt even if a different output format is specified?

eliottBo · 2025-12-03T16:21:36Z

Thanks, that looks very good to me. I have one additional question, probably because I have no experience with it: independent of any --gtf or --tsv setting, will it always print to stdout, i.e. can you capture the stats with > ${prefix}.txt even if a different output format is specified?

I thought about always capturing the output in the .txt file but from a user point of view I thought to get only the output you want would be better. As it is now the user can definately get the stdout which will end up in the .txt file. The combination of all output is possible. If, for example, the user is just interested in the .tsv or .gtf it is maybe not necessary to always create this .txt file with the stdout. But if you think it is better to always have it, I can add it by default.

nschan · 2025-12-03T16:46:23Z

No, that is not what I meant, I only wanted to clarify if whatshap stats will always output the same format to stdout independent of the format specified.
However, this made me remember something: nf-core guidelines recommend using tee when capturing stdout (https://nf-co.re/docs/guidelines/components/modules#capturing-stdout-and-stderr). I think this is more of a "nice to do right" thing, but maybe it would be useful?

nschan

I think it might be a bit confusing that now the default behaviour is not producing any output, as all include_* values are defaulting to false. Since this is documented, I am okay with it, but maybe it would be better to have some default output..

fellen31

Some suggestions! Nice job!

fellen31 · 2025-12-07T16:00:45Z

modules/nf-core/whatshap/stats/main.nf

+    tuple val(meta), path("${prefix}_whap_stats.tsv"),                         emit: tsv,   optional: true
+    tuple val(meta), path("${prefix}_whap_stats.gtf"),                         emit: gtf,   optional: true
+    tuple val(meta), path("${prefix}_whap_stats_block.txt"),                   emit: block, optional: true
+    tuple val(meta), path("${prefix}_whap_stats.txt"),                         emit: txt,   optional: true


I suggest giving the user of the module the opportunity to choose to include e.g. _whap_stats via the prefix, if they want to, rather than hardcoding it. Although we would need some distinction between the block (which is also tab-separated) and txt output (calling the txt log for example, which I'd argue that it is, especially when running with the --debug flag), if it's kept.

Suggested change

tuple val(meta), path("${prefix}_whap_stats.tsv"), emit: tsv, optional: true

tuple val(meta), path("${prefix}_whap_stats.gtf"), emit: gtf, optional: true

tuple val(meta), path("${prefix}_whap_stats_block.txt"), emit: block, optional: true

tuple val(meta), path("${prefix}_whap_stats.txt"), emit: txt, optional: true

tuple val(meta), path("${prefix}.tsv"), emit: tsv, optional: true

tuple val(meta), path("${prefix}.gtf"), emit: gtf, optional: true

tuple val(meta), path("${prefix}.txt"), emit: block, optional: true

tuple val(meta), path("${prefix}.log"), emit: log, optional: true

fellen31 · 2025-12-07T16:01:00Z

modules/nf-core/whatshap/stats/main.nf

+        $output_gtf \\
+        $output_block \\
+        $vcf \\
+        $output_txt \\


Suggested change

$output_txt \\

$output_txt

fellen31 · 2025-12-07T16:03:47Z

modules/nf-core/whatshap/stats/main.nf

+    def output_tsv   = include_tsv_output  ? "--tsv ${prefix}_whap_stats.tsv"              : ''
+    def output_gtf   = include_gtf_output  ? "--gtf ${prefix}_whap_stats.gtf"              : ''
+    def output_block = inlude_block_output ? "--block-list ${prefix}_whap_stats_block.txt" : ''
+    def output_txt   = include_txt_output  ? "> ${prefix}_whap_stats.txt"                  : ''


I think having it like this is OK, but I would also be down to scrap the stdout output altogether. I think it's more of a log rather than an output really. No one in their right mind would choose to process that over the TSV 😅 (used by e.g. MultiQC). The printed output would still available in the stdout files.

If kept, since the tool will aways write this to stdout, regardless of whether any of the other output formats are selected, I think it would be okay to always output this log as default as well. The user doesn't have to use it if they don't want to.

fellen31 · 2025-12-07T16:18:33Z

modules/nf-core/whatshap/stats/main.nf

+    def output_tsv   = include_tsv_output  ? "--tsv ${prefix}_whap_stats.tsv" : ''
+    def output_gtf   = include_gtf_output  ? "--gtf ${prefix}_whap_stats.gtf" : ''
+    def output_block = inlude_block_output ? "--block-list ${prefix}_whap_stats_block.txt" : ''
+    def output_txt   = include_txt_output  ? "> ${prefix}_whap_stats.txt" : ''
+    """
+    touch ${prefix}_whap_stats.tsv
+    touch ${prefix}_whap_stats.gtf
+    touch ${prefix}_whap_stats_block.txt
+    touch ${prefix}_whap_stats.txt


Something like this would make the stub section mimic the script section correctly.

Suggested change

def output_tsv = include_tsv_output ? "--tsv ${prefix}_whap_stats.tsv" : ''

def output_gtf = include_gtf_output ? "--gtf ${prefix}_whap_stats.gtf" : ''

def output_block = inlude_block_output ? "--block-list ${prefix}_whap_stats_block.txt" : ''

def output_txt = include_txt_output ? "> ${prefix}_whap_stats.txt" : ''

"""

touch ${prefix}_whap_stats.tsv

touch ${prefix}_whap_stats.gtf

touch ${prefix}_whap_stats_block.txt

touch ${prefix}_whap_stats.txt

def tsv_touch_cmd = include_tsv_output ? "--tsv ${prefix}_whap_stats.tsv" : ''

def gtf_touch_cmd = include_gtf_output ? "touch ${prefix}.tsv" : ''

def block_touch_cmd = inlude_block_output ? "touch ${prefix}.txt" : ''

def log_touch_cmd = include_txt_output ? "touch ${prefix}.log" : ''

"""

echo $args

$tsv_touch_cmd

$gtf_touch_cmd

$block_touch_cmd

$txt_touch_cmd

eliottBo added 8 commits November 28, 2025 16:35

Create module whatshap/stats

3679ae4

Add containers

e2f9553

Modify module according to TODO

843c1bd

Remove TODO and Harshil alignment

31df5fa

Remove TODO comment in environment.yml

e55ed01

Simplify the module to output only the tsv file and to capture the ou…

2bea721

…tput in a txt file

Create test and snapshot

842ee43

Modify the meta.yml

6f8a4f2

eliottBo self-assigned this Dec 2, 2025

eliottBo and others added 9 commits December 2, 2025 10:50

Merge branch 'master' into 5787-new-module-whatshap-stats

cb84001

Fix topics section in meta.yml

d9b96db

Merge branch '5787-new-module-whatshap-stats' of github.com:eliottBo/…

43d18fe

…modules into 5787-new-module-whatshap-stats

Merge branch 'master' into 5787-new-module-whatshap-stats

e387475

Prettier formatting

7fb7494

Merge branch '5787-new-module-whatshap-stats' of github.com:eliottBo/…

2083d09

…modules into 5787-new-module-whatshap-stats

Remove trailing lines

07b762c

fix formatting in test file

484e9cf

Merge branch 'master' into 5787-new-module-whatshap-stats

7b1de10

eliottBo marked this pull request as ready for review December 2, 2025 14:11

nschan reviewed Dec 2, 2025

View reviewed changes

modules/nf-core/whatshap/stats/main.nf Outdated Show resolved Hide resolved

eliottBo and others added 5 commits December 2, 2025 15:39

Add prefix to output file tsv and txt

7fc7b94

Modify main to adapt for output types. Modify meta and tests accordingly

8daa6e2

Change test file and update tests

f7005af

Merge branch '5787-new-module-whatshap-stats' of github.com:eliottBo/…

c95952f

…modules into 5787-new-module-whatshap-stats

Merge branch 'master' into 5787-new-module-whatshap-stats

ec33419

Merge branch 'master' into 5787-new-module-whatshap-stats

5f64f2a

nschan approved these changes Dec 4, 2025

View reviewed changes

fellen31 mentioned this pull request Dec 5, 2025

Phasing with SVs and before annotation genomic-medicine-sweden/nallo#800

Open

10 tasks

fellen31 reviewed Dec 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

5787 new module whatshap/stats #9490

5787 new module whatshap/stats #9490

eliottBo commented Dec 2, 2025

Uh oh!

Uh oh!

nschan commented Dec 2, 2025

Uh oh!

eliottBo commented Dec 3, 2025 •

edited

Loading

Uh oh!

nschan commented Dec 3, 2025

Uh oh!

eliottBo commented Dec 3, 2025 •

edited

Loading

Uh oh!

nschan commented Dec 3, 2025

Uh oh!

nschan left a comment

Uh oh!

fellen31 left a comment

Uh oh!

fellen31 Dec 7, 2025

Uh oh!

fellen31 Dec 7, 2025

Uh oh!

fellen31 Dec 7, 2025

Uh oh!

fellen31 Dec 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    def output_tsv   = include_tsv_output  ? "--tsv ${prefix}_whap_stats.tsv" : ''
-    def output_gtf   = include_gtf_output  ? "--gtf ${prefix}_whap_stats.gtf" : ''
-    def output_block = inlude_block_output ? "--block-list ${prefix}_whap_stats_block.txt" : ''
-    def output_txt   = include_txt_output  ? "> ${prefix}_whap_stats.txt" : ''
-    """
-    touch ${prefix}_whap_stats.tsv
-    touch ${prefix}_whap_stats.gtf
-    touch ${prefix}_whap_stats_block.txt
-    touch ${prefix}_whap_stats.txt
+    def tsv_touch_cmd   = include_tsv_output  ? "--tsv ${prefix}_whap_stats.tsv" : ''
+    def gtf_touch_cmd   = include_gtf_output  ? "touch ${prefix}.tsv" : ''
+    def block_touch_cmd = inlude_block_output ? "touch ${prefix}.txt" : ''
+    def log_touch_cmd   = include_txt_output  ? "touch ${prefix}.log" : ''
+    """
+    echo $args
+    $tsv_touch_cmd
+    $gtf_touch_cmd
+    $block_touch_cmd
+    $txt_touch_cmd

5787 new module whatshap/stats #9490

Are you sure you want to change the base?

5787 new module whatshap/stats #9490

Conversation

eliottBo commented Dec 2, 2025

PR checklist

Uh oh!

Uh oh!

nschan commented Dec 2, 2025

Uh oh!

eliottBo commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nschan commented Dec 3, 2025

Uh oh!

eliottBo commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nschan commented Dec 3, 2025

Uh oh!

nschan left a comment

Choose a reason for hiding this comment

Uh oh!

fellen31 left a comment

Choose a reason for hiding this comment

Uh oh!

fellen31 Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

fellen31 Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

fellen31 Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

fellen31 Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eliottBo commented Dec 3, 2025 •

edited

Loading

eliottBo commented Dec 3, 2025 •

edited

Loading