Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- ([#2415](https://github.com/nf-core/tools/pull/2415#issuecomment-1709847086)) Add autoMounts for apptainer.
- Remove `igenomes_base` from the schema, so that nf-validation doesn't create a file path and throw errors offline for s3 objects.
- Update Gitpod profile resources to reflect base environment settings.
- ([#2430](https://github.com/nf-core/tools/pull/2430)) - Use INITIALISE subworkflow to start pipeline and validate parameters etc.

### Download

Expand Down
46 changes: 0 additions & 46 deletions nf_core/pipeline-template/lib/WorkflowMain.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,8 @@
// This file holds several functions specific to the main.nf workflow in the {{ name }} pipeline
//

import nextflow.Nextflow

class WorkflowMain {

//
// Citation string for pipeline
//
public static String citation(workflow) {
return "If you use ${workflow.manifest.name} for your analysis please cite:\n\n" +
// TODO nf-core: Add Zenodo DOI for pipeline after first release
//"* The pipeline\n" +
//" https://doi.org/10.5281/zenodo.XXXXXXX\n\n" +
"* The nf-core framework\n" +
" https://doi.org/10.1038/s41587-020-0439-x\n\n" +
"* Software dependencies\n" +
" https://github.com/${workflow.manifest.name}/blob/master/CITATIONS.md"
}


//
// Validate parameters and print summary to screen
//
public static void initialise(workflow, params, log) {

// Print workflow version and exit on --version
if (params.version) {
String workflow_version = NfcoreTemplate.version(workflow)
log.info "${workflow.manifest.name} ${workflow_version}"
System.exit(0)
}

// Check that a -profile or Nextflow config has been provided to run the pipeline
NfcoreTemplate.checkConfigProvided(workflow, log)

// Check that conda channels are set-up correctly
if (workflow.profile.tokenize(',').intersect(['conda', 'mamba']).size() >= 1) {
Utils.checkCondaChannels(log)
}

// Check AWS batch settings
NfcoreTemplate.awsBatch(workflow, params)

// Check input has been provided
if (!params.input) {
Nextflow.error("Please provide an input samplesheet to the pipeline e.g. '--input samplesheet.csv'")
}
}

{%- if igenomes %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the igenomes bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

//
// Get attribute from genome config file e.g. fasta
Expand Down
20 changes: 3 additions & 17 deletions nf_core/pipeline-template/main.nf
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,7 @@ params.fasta = WorkflowMain.getGenomeAttribute(params, 'fasta')
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/

include { validateParameters; paramsHelp } from 'plugin/nf-validation'

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

def citation = '\n' + WorkflowMain.citation(workflow) + '\n'
def String command = "nextflow run ${workflow.manifest.name} --input samplesheet.csv --genome GRCh37 -profile docker"
log.info logo + paramsHelp(command) + citation + NfcoreTemplate.dashedLine(params.monochrome_logs)
System.exit(0)
}

// Validate input parameters
if (params.validate_params) {
validateParameters()
}

WorkflowMain.initialise(workflow, params, log)
include { INITIALISE } from './subworkflows/nf-core/initialise/main.nf'

/*
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand All @@ -60,6 +44,8 @@ include { {{ short_name|upper }} } from './workflows/{{ short_name }}'
// WORKFLOW: Run main {{ name }} analysis pipeline
//
workflow {{ prefix_nodash|upper }}_{{ short_name|upper }} {
log.info NfcoreTemplate.logo(workflow, params.monochrome_logs)
INITIALISE(params.version, params.help, params.validate_params)
{{ short_name|upper }} ()
}

Expand Down
9 changes: 9 additions & 0 deletions nf_core/pipeline-template/modules.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@
"installed_by": ["modules"]
}
}
},
"subworkflows": {
"nf-core": {
"initialise": {
"branch": "master",
"git_sha": "2a3d1b469385914c4eeca70f36bc378473049d93",
"installed_by": ["subworkflows"]
}
}
}
}
}
Expand Down
68 changes: 68 additions & 0 deletions nf_core/pipeline-template/subworkflows/nf-core/initialise/main.nf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions nf_core/pipeline-template/subworkflows/nf-core/initialise/meta.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 4 additions & 10 deletions nf_core/pipeline-template/workflows/pipeline.nf
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/

include { paramsSummaryLog; paramsSummaryMap } from 'plugin/nf-validation'

def logo = NfcoreTemplate.logo(workflow, params.monochrome_logs)
def citation = '\n' + WorkflowMain.citation(workflow) + '\n'
def summary_params = paramsSummaryMap(workflow)

// Print parameter summary log to screen
log.info logo + paramsSummaryLog(workflow) + citation

Workflow{{ short_name[0]|upper }}{{ short_name[1:] }}.initialise(params, log)
include { paramsSummaryMap } from 'plugin/nf-validation'

/*
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! sounds good then :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


workflow {{ short_name|upper }} {

ch_versions = Channel.empty()
Expand Down
9 changes: 9 additions & 0 deletions tests/subworkflows/remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def test_subworkflows_remove_uninstalled_subworkflow(self):

def test_subworkflows_remove_subworkflow(self):
"""Test removing subworkflow and all it's dependencies after installing it"""
# Remove initialise subworkflow for clean slate
self.subworkflow_remove.remove("initialise")

# Install subworkflow
self.subworkflow_install.install("bam_sort_stats_samtools")

subworkflow_path = Path(self.subworkflow_install.dir, "subworkflows", "nf-core")
Expand All @@ -34,6 +38,10 @@ def test_subworkflows_remove_subworkflow(self):

def test_subworkflows_remove_subworkflow_keep_installed_module(self):
"""Test removing subworkflow and all it's dependencies after installing it, except for a separately installed module"""
# Remove initialise subworkflow for clean slate
self.subworkflow_remove.remove("initialise")

# Install subworkflow and module
self.subworkflow_install.install("bam_sort_stats_samtools")
self.mods_install.install("samtools/index")

Expand All @@ -44,6 +52,7 @@ def test_subworkflows_remove_subworkflow_keep_installed_module(self):

mod_json_before = ModulesJson(self.pipeline_dir).get_modules_json()
assert self.subworkflow_remove.remove("bam_sort_stats_samtools")

mod_json_after = ModulesJson(self.pipeline_dir).get_modules_json()

assert Path.exists(subworkflow_path) is False
Expand Down
3 changes: 3 additions & 0 deletions tests/subworkflows/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ def test_update_with_config_dont_update(self):

def test_update_with_config_fix_all(self):
"""Fix the version of all nf-core subworkflows"""
# Remove intialise
self.subworkflow_remove.remove("initialise")

# Install subworkflow at the latest version
assert self.subworkflow_install.install("fastq_align_bowtie2")

Expand Down