Skip to content

"change_name" process and configuration of output file name#177

Merged
muffato merged 6 commits into
hhoang-read-coveragefrom
hhoang-read-coverage_change_name
Jul 9, 2025
Merged

"change_name" process and configuration of output file name#177
muffato merged 6 commits into
hhoang-read-coveragefrom
hhoang-read-coverage_change_name

Conversation

@muffato

@muffato muffato commented Jul 3, 2025

Copy link
Copy Markdown
Member

I found it a bit confusing to have so many channels of BAM files in subworkflows/local/convert_stats.nf, and also the fact that we define the name of the output files multiple times in conf/modules.config.

The core of my proposal is to pass all BAM files through change_name, define the output name there just once, and then to make all subsequent steps use something like input.baseName as their "task prefix".

To avoid having to make a channel that combines BAM files with the input fasta, it can simply be passed as an extra input channel in change_name.
The channels used in the module are:

  1. bam: list of input BAM files
  2. ch_bams_for_renaming: list of BAM files, now compressed with crumble if requested. ready to go through change_name
  3. ch_renamed_bams. Same BAM files as ch_bams_for_renaming but now renamed.
    1. ch_renamed_bams is used in the conversion to CRAM, blobtk_depth and all the samtools *stats processes
    2. There are two places where I instead use CHANGE_NAME.out.file. This is because CHANGE_NAME.out.file is just [meta, bam] whereas as is ch_renamed_bams is [meta, bam, []]. Maybe you'd want to introduce another variable name to make the distinction clearer ? Or do a map from ch_renamed_bams on the fly ?

I hoped publishing the files would be trivial but it wasn't.
Because CHANGE_NAME is now used for everything, it'll make a BAM file even when outfmt is set to cram. I therefore fully removed CHANGE_NAME from the publishing rule with the objective of doing something like this:

  • CRAM and CRAM index files are published from SAMTOOLS_CRAM
  • BAM and BAM index files are published from SAMTOOLS_INDEX

The problems were that:

  1. SAMTOOLS_CRAM is an instance of samtools/view, which puts the input BAM file in an output channel. So I had to add ((task.process =~ /SAMTOOLS_CRAM/) && filename.endsWith(".bam")) to exclude it
  2. I had to patch SAMTOOLS_INDEX to output the input file as well.

So overall, I think it's maybe a bit simpler in terms of channels and processes in the sub-workflow, but publishing the output files is more complex and maybe harder to maintain.

What do you think ? Is it worth it at all ?

PR checklist

  • 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 pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile 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).

muffato added 3 commits July 2, 2025 15:47
- CHANGE_NAME is always called and is the only place where the published
  file name is set
- All downstream modules use the renamed BAMs as input and copy its name
  to the output
We don't publish anything from CHANGE_NAME itself. Instead we make
SAMTOOLS_INDEX publish its input file.
@muffato muffato requested a review from sainsachiko July 3, 2025 16:09
@muffato muffato self-assigned this Jul 3, 2025
@sanger-tolsoft

Copy link
Copy Markdown
Contributor

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.2.1.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

Base automatically changed from hhoang-read-coverage_review to hhoang-read-coverage July 4, 2025 09:39
@sainsachiko

Copy link
Copy Markdown
Contributor

Hi @muffato, I have added a commit to revert ch_for_stats (both file and index) for statistics processes as we discussed. I also edited the condition in modules.config to publish cram index when provide header (.crai is accidentally ignored in my modifications in hhoang-read-coverage branch)

@sainsachiko

Copy link
Copy Markdown
Contributor

@sanger-tolsoft fix linting

@muffato muffato merged commit 6119963 into hhoang-read-coverage Jul 9, 2025
5 checks passed
@muffato muffato deleted the hhoang-read-coverage_change_name branch July 9, 2025 21:04
@muffato muffato added this to the v1.4.0 milestone Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants