rnaseq icon indicating copy to clipboard operation
rnaseq copied to clipboard
verified publisher icon nf-core

Use channel topic for tool versions

Open bentsherman opened this issue 2 years ago • 10 comments

This PR refactors the pipeline to use an experimental topic channel (https://github.com/nextflow-io/nextflow/pull/4459) to collect the tool versions.

~~The topic channel works, but multiqc is failing with this error:~~

bentsherman avatar Nov 09 '23 16:11 bentsherman

nf-core lint overall result: Failed :x:

Posted for pipeline commit 8f7e5bd

+| ✅ 144 tests passed       |+
#| ❔   6 tests were ignored |#
!| ❗   4 tests had warnings |!
-| ❌   1 tests failed       |-

:x: Test failures:

  • actions_ci - Minimum pipeline NF version '23.04.0' is not tested in '.github/workflows/ci.yml'

:heavy_exclamation_mark: Test warnings:

  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in WorkflowRnaseq.groovy: Optionally add in-text citation tools to this list.

:grey_question: Tests ignored:

  • files_unchanged - File ignored due to lint config: assets/email_template.html
  • files_unchanged - File ignored due to lint config: assets/email_template.txt
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/rnaseq/rnaseq/.github/workflows/awstest.yml
  • multiqc_config - multiqc_config

:white_check_mark: Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-12-12 20:14:54

github-actions[bot] avatar Nov 09 '23 16:11 github-actions[bot]

-225 lines!

pditommaso avatar Nov 09 '23 16:11 pditommaso

I think you managed to hit the small window when the MultiQC bioconda build was broken. If you try again now it should be fine 🤞🏻

ewels avatar Nov 12 '23 11:11 ewels

This is awesome! Do you think it'd be possible to go even further and use a topic to pick up all of the MultiQC inputs, not just the versions?

ewels avatar Nov 12 '23 11:11 ewels

This is indeed awesome!! 🤩

When this is out in a release, we will first need to bump the minimum version of NF on nf-core/modules as well as in pipeline repos. Might need to delay a little to enable users to bump their NF version installs.

Do you think it'd be possible to go even further and use a topic to pick up all of the MultiQC inputs, not just the versions?

Presumably, you would need to explicitly add topic: multiqc within all relevant modules for this to work?

output:
path "<SOME_MULTIQC_FILE", topic: multiqc

Is it possible to use emit and topic together in an output channel? There may be some instances where you want to use the output of a module in a workflow independent of a topic.

drpatelh avatar Nov 13 '23 12:11 drpatelh

I think you managed to hit the small window when the MultiQC bioconda build was broken

Okay I deleted the conda environment for multiqc but still got the error with a new build... does conda use a local cache that could be preventing a fresh download?

When this is out in a release, we will first need to bump the minimum version of NF on nf-core/modules as well as in pipeline repos.

This will be the challenge. It might be better to only patch the nf-core modules for the rnaseq pipeline and update the modules after another stable release (or two).

Is it possible to use emit and topic together in an output channel? There may be some instances where you want to use the output of a module in a workflow independent of a topic.

~~Currently it is not possible, not sure how difficult it would be to support. I can look at the multiqc inputs and see if a topic channel would be appropriate.~~

bentsherman avatar Nov 13 '23 13:11 bentsherman

In any case, I tested rnaseq on master and it fails with the same error, so that at least makes me confident that there's no issue with the topic channels

bentsherman avatar Nov 13 '23 15:11 bentsherman

As for the multiqc inputs, a topic might not be the best choice because it is not obvious that the corresponding process outputs are intended solely for multiqc.

For example, the FASTP process:

    output:
    tuple val(meta), path('*.fastp.fastq.gz') , optional:true, emit: reads
    tuple val(meta), path('*.json')           , emit: json
    tuple val(meta), path('*.html')           , emit: html
    tuple val(meta), path('*.log')            , emit: log
    path "versions.yml"                       , topic: versions
    tuple val(meta), path('*.fail.fastq.gz')  , optional:true, emit: reads_fail
    tuple val(meta), path('*.merged.fastq.gz'), optional:true, emit: reads_merged

Some of these outputs are sent to multiqc, but you couldn't tell from the process definition. I think forcing a channel topic here would be more trouble than it's worth.

bentsherman avatar Nov 13 '23 16:11 bentsherman

Shall we move this forward? Channel topics are available in 23.11.0-edge. Let me know how you want to handle it. Or maybe someone from nf-core can take it forward, don't really need me anymore 😄

Is it possible to use emit and topic together in an output channel? There may be some instances where you want to use the output of a module in a workflow independent of a topic.

I was wrong about this, you can use emit and topic together. So if you guys want you can add emit: ch_versions back to the process outputs.

Also would love to hear back about my comment regarding the use of env output versus the experimental cmd output. If you prefer the env approach then we can implement that right away, for the cmd approach we can implement separately if/when it is merged.

bentsherman avatar Dec 06 '23 13:12 bentsherman

I updated the CI and tests are passing now. The failing tests should work if they use 23.11.0-edge, but not sure how you wanted to manage that.

bentsherman avatar Dec 12 '23 20:12 bentsherman