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

Add check if parameters are specified via .config file to pipeline template #2184

Open
grst opened this issue Feb 16, 2023 · 6 comments
Open
Labels
enhancement infrastructure template nf-core pipeline/component template

Comments

@grst
Copy link
Member

grst commented Feb 16, 2023

Description of feature

Specifying parameters via -c custom.config can make problems with DSL2 workflows. The corresponding documentation was added in #2173.

I'm afraid there's still a considerable number of users following this practice, and in the worst case this may lead to incorrect results because parameters are silently ignored. Having a loud warning (or even error) when a workflow with a custom config file is started would be great!

I prototyped this snipped, and it seems to work in principle. However, it would need some tuning that it doesn't raise an error for the config file that defines the default parameters.

def check_config_files(workflow) {
    for (config_file in workflow.configFiles) {
        if (file(config_file).text.contains("parameters")) {
            println "WARNING: Do not specify parameters via a .config file."
        }
    }
}

workflow {
    check_config_files(workflow)
}

Maybe someone can take this on at the hackathon?

CC @jfy133

@grst grst added enhancement template nf-core pipeline/component template labels Feb 16, 2023
@github-project-automation github-project-automation bot moved this to Todo (Hackathon General) in nf-core Hackathon March 2023 Mar 21, 2023
@drpatelh
Copy link
Member

parameters -> params

The ideal situation here would be to load each of these configs and see whether the params object is empty or not. Looking for strings like params could go wrong too?

@drpatelh
Copy link
Member

drpatelh commented Mar 23, 2023

This also assumes users will read STDOUT which they probably won't in the Cloud.

@drpatelh
Copy link
Member

See nf-core/rnaseq#972 for how it has been added to rnaseq and viralrecon for now. We can take what we want from this and/or update too.

@grst
Copy link
Member Author

grst commented Mar 23, 2023

This also assumes users will read STDOUT which they probably won't in the Cloud.

unless it aborts the pipeline? But aborting would definitely be too risky when doing a simple string search.

See nf-core/rnaseq#972 for how it has been added to rnaseq and viralrecon for now. We can take what we want from this and/or update too.

👍, but this also leads to a few false-positives? E.g. providing process withName: via config file is completely legit.

Loading the config files would be perfect... is there a way to access individual config files in nextflow?

@maxulysse
Copy link
Member

yes, but I think the warning message is clear enough

@grst
Copy link
Member Author

grst commented Mar 23, 2023

yes, but I think the warning message is clear enough

agree, and it's already better than my naive check for "params".

My point was: if we had a reliable way of detecting that params were specified via config file, I'd consider a failure instead of a warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement infrastructure template nf-core pipeline/component template
Projects
No open projects
Status: No status
Development

No branches or pull requests

4 participants