Use INITIALISE subworkflow instead of WorkflowMain.initialise() #2430

adamrtalbot · 2023-09-21T12:39:27Z

Pipeline template: Use INIITIALISE subworkflow instead of WorkflowMain.initialise()

Changes:

Guts WorkflowMain, particularly the initialise() method
Replaces it with INIITIALISE subworkflow
Should be easier to maintain via nf-core/modules (more regular updates)
Uses more native Nextflow for bioinformatics developers to follow

Should incidentally fix https://github.com/nf-core/tools/issues/2289>

PR checklist

This comment contains a description of changes (with reason)
CHANGELOG.md is updated
If you've fixed a bug or added code that should be tested, add tests!
Documentation in docs is updated

…n.initialise() Changes: - Guts WorkflowMain, particularly the initialise() method - Replaces it with INIITIALISE subworkflow - Should be easier to maintain via nf-core/modules (more regular updates) - Uses more native Nextflow for bioinformatics developers to follow Should incidentally fix nf-core#2289

adamrtalbot · 2023-09-21T13:04:56Z

nf_core/pipeline-template/workflows/pipeline.nf

@@ -59,6 +50,9 @@ include { CUSTOM_DUMPSOFTWAREVERSIONS } from '../modules/nf-core/custom/dumpsoft
 // Info required for completion email and summary
 def multiqc_report = []

+// Get map of parameter values
+def summary_params = paramsSummaryMap(workflow)


We could possibly add this to the INITIALISE subworkflow to make everything central.

INITIALISE is meant to be used in the main.nf isn't it? I think it will be difficult to use it here.

It could work anywhere, but we would have to re-work structure slightly for the scoping to be correct for this particular use case. But it's hardly a massive problem having it here anyway. It would be cleaner to not have anything outside the workflow scope in this file, e.g. global scope stuff only happens in ./main.nf.

I see! sounds good then :)

I'd remove even more the workflow to be honest, but that's for the next release, we're too early to move so fast I'm afraid

Would you like to add this modification if it is a fast change and merge the PR for this release?

codecov · 2023-09-21T17:28:12Z

Codecov Report

Merging #2430 (eab1442) into dev (6b20a81) will decrease coverage by 0.02%.
Report is 37 commits behind head on dev.
The diff coverage is 20.83%.

❗ Current head eab1442 differs from pull request most recent head 31a47a8. Consider uploading reports for the commit 31a47a8 to get more accurate results

@@            Coverage Diff             @@
##              dev    #2430      +/-   ##
==========================================
- Coverage   71.07%   71.06%   -0.02%     
==========================================
  Files          87       87              
  Lines        9431     9431              
==========================================
- Hits         6703     6702       -1     
- Misses       2728     2729       +1

Files Changed	Coverage Δ
nf_core/download.py	`59.35% <20.83%> (ø)`

... and 1 file with indirect coverage changes

mirpedrol · 2023-09-25T08:25:53Z

nf_core/pipeline-template/main.nf

-
-// Print help message if needed
-if (params.help) {
-    def logo = NfcoreTemplate.logo(workflow, params.monochrome_logs)


The subworkflow INITIALISE does not contain the nf-core logo, we should add it or keep it here.

I've added it back, but it now always appears. Options:

Leave it in main.nf, let a pipeline developer handle it as they want

Add to INITIALISE, with an optional flag so it only appears with --help or pipeline run (a bit annoying with the monochrome logs stuff, but doable).

For now, adding NfcoreTemplate.logo to ./main.nf is probably 'good enough'.

I agree with adding it to ./main.nf.
The only problem I can see is with retrieving the version from CLI, but to be fair it was already printing some text that must be stripped, so it doesn't look as a big change to me.

I'm just having a look now and as long as you switch around the order of functions correctly I think it will be fine(ish).

mirpedrol · 2023-09-25T08:25:59Z

nf_core/pipeline-template/workflows/pipeline.nf

@@ -59,6 +50,9 @@ include { CUSTOM_DUMPSOFTWAREVERSIONS } from '../modules/nf-core/custom/dumpsoft
 // Info required for completion email and summary
 def multiqc_report = []

+// Get map of parameter values
+def summary_params = paramsSummaryMap(workflow)


INITIALISE is meant to be used in the main.nf isn't it? I think it will be difficult to use it here.

maxulysse · 2023-09-25T09:27:21Z

nf_core/pipeline-template/lib/WorkflowMain.groovy

-            Nextflow.error("Please provide an input samplesheet to the pipeline e.g. '--input samplesheet.csv'")
-        }
-    }
-
    {%- if igenomes %}


what about the igenomes bit?

I haven't touched igenomes - you still need to use WorkflowMain.getGenomeAttribute for now.

nf-core/modules#3864

Since igenomes is optional in case you are creating a customised template, I think the best is to keep it in this groovy function

I am on a one man crusade to remove everything from lib/. But yes, I'm not sure it's appropriate within INITIALISE. Haven't got a good solution for this yet 🤔

maxulysse

looking good

adamrtalbot · 2023-09-25T11:14:20Z

My personal feeling is we should merge this one after the next release and upcoming hackathon (Oct 2023). That way we can spend some time refining it before the next major release. Do you agree @maxulysse @mirpedrol?

adamrtalbot · 2024-05-22T15:07:30Z

No longer needed

adamrtalbot force-pushed the use_initialise_Subworkflow branch from a502001 to cf14aa1 Compare September 21, 2023 12:40

Template fixes

e9be029

adamrtalbot requested review from mirpedrol and mashehu September 21, 2023 13:04

adamrtalbot commented Sep 21, 2023

View reviewed changes

adamrtalbot added 4 commits September 21, 2023 16:12

Made sure it matches existing template

cc01dd0

fixup

61fbeb8

Update tests to remove INITIALISE subworkflow before testing

8153dc4

CHANGELOG

7e307e6

adamrtalbot requested a review from maxulysse September 21, 2023 15:26

Test for updating modules fixed

83ccc5e

initialise-subworkflow-update

eab1442

mirpedrol reviewed Sep 25, 2023

View reviewed changes

Re-add nf-core logo back to template

31a47a8

maxulysse reviewed Sep 25, 2023

View reviewed changes

maxulysse approved these changes Sep 25, 2023

View reviewed changes

maxulysse mentioned this pull request Sep 25, 2023

Add igenomes dedicated utils nf-core/modules#3864

Open

mirpedrol approved these changes Sep 25, 2023

View reviewed changes

adamrtalbot closed this May 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use INITIALISE subworkflow instead of WorkflowMain.initialise() #2430

Use INITIALISE subworkflow instead of WorkflowMain.initialise() #2430

adamrtalbot commented Sep 21, 2023 •

edited

adamrtalbot Sep 21, 2023

mirpedrol Sep 25, 2023

adamrtalbot Sep 25, 2023

mirpedrol Sep 25, 2023

maxulysse Sep 25, 2023

mirpedrol Sep 25, 2023

codecov bot commented Sep 21, 2023 •

edited

mirpedrol Sep 25, 2023

adamrtalbot Sep 25, 2023

mirpedrol Sep 25, 2023

adamrtalbot Sep 25, 2023

mirpedrol Sep 25, 2023

maxulysse Sep 25, 2023

adamrtalbot Sep 25, 2023

maxulysse Sep 25, 2023

mirpedrol Sep 25, 2023

adamrtalbot Sep 25, 2023

maxulysse left a comment

adamrtalbot commented Sep 25, 2023

adamrtalbot commented May 22, 2024

Use INITIALISE subworkflow instead of WorkflowMain.initialise() #2430

Use INITIALISE subworkflow instead of WorkflowMain.initialise() #2430

Conversation

adamrtalbot commented Sep 21, 2023 • edited

PR checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 21, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxulysse left a comment

Choose a reason for hiding this comment

adamrtalbot commented Sep 25, 2023

adamrtalbot commented May 22, 2024

adamrtalbot commented Sep 21, 2023 •

edited

codecov bot commented Sep 21, 2023 •

edited