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

Summary lib function prints false as different to the schema when no default #971

Closed
ewels opened this issue Mar 25, 2021 · 1 comment
Closed
Labels
bug Something isn't working high-priority schema template nf-core pipeline/component template

Comments

@ewels
Copy link
Member

ewels commented Mar 25, 2021

The NfcoreSchema lib function params_summary_map prints quite a lot of values for the summary when they do not vary from the schema.

  • If a parameter in nextflow.config is set to null or false and the pipeline schema does not have a default parameter set
  • All memory and time units (string representation output is different to schema)

Example output from the nf-core/methylseq pipeline using -profile test:

------------------------------------------------------
                                        ,--./,-.
        ___     __   __   __   ___     /,-._.--~'
  |\ | |__  __ /  ` /  \ |__) |__         }  {
  | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                        `._,._,'
  nf-core/methylseq v1.6
------------------------------------------------------

Core Nextflow options
    runName                        : awesome_minsky
    containerEngine                : docker
    container                      : nfcore/methylseq:1.6
    launchDir                      : /Users/ewels/GitHub/nf-core/methylseq/testing
    workDir                        : /Users/ewels/GitHub/nf-core/methylseq/testing/work
    projectDir                     : /Users/ewels/GitHub/nf-core/methylseq
    userName                       : ewels
    profile                        : test,docker
    configFiles                    : /Users/ewels/GitHub/nf-core/methylseq/nextflow.config

Input/output options
    input                          : null
    single_end                     : true
    email                          : false

Reference genome options
    genome                         : false
    fasta                          : https://github.com/nf-core/test-datasets/raw/methylseq/reference/genome.fa
    fasta_index                    : https://github.com/nf-core/test-datasets/raw/methylseq/reference/genome.fa.fai
    bismark_index                  : false
    bwa_meth_index                 : false

Bismark options
    meth_cutoff                    : false
    known_splices                  : false
    minins                         : false
    maxins                         : false
    bismark_align_cpu_per_multicore: null
    bismark_align_mem_per_multicore: null

Generic options
    email_on_fail                  : false
    max_multiqc_email_size         : 25 MB
    multiqc_config                 : false

Max job request options
    max_cpus                       : 2
    max_memory                     : 6 GB
    max_time                       : 2d

Institutional config options
    config_profile_name            : Test profile
    config_profile_description     : Minimal test dataset to check pipeline function
    config_profile_contact         : false
    config_profile_url             : false
    project                        : false
    cluster_options                : false

[Only displaying parameters that differ from pipeline default]
------------------------------------------------------

I think that a lot of this is is because the schema default for strings is set to an empty string if it is not set:

if (schema_value == null) {
if (param_type == 'boolean') {
schema_value = false
}
if (param_type == 'string') {
schema_value = ''
}
if (param_type == 'integer') {
schema_value = 0
}

This then means that the default doesn't match it in this block:

if (params_value != schema_value) {
sub_params.put("$param", params_value)
}

I wonder if we should just delete the first block (I don't think that should break anything, right?) and standardise that unset parameters should be defined as null in nextflow.config. I had a discussion with @drpatelh in Slack about this the other day I think too.

For the memory & time, I wonder if we just try and parse all strings as memory / time units - if it succeeds for both the param and the default we can save them as the correct unit and the default matching should work.

@ewels ewels added bug Something isn't working template nf-core pipeline/component template high-priority schema labels Mar 25, 2021
@ewels ewels mentioned this issue Mar 25, 2021
3 tasks
@ewels
Copy link
Member Author

ewels commented Mar 26, 2021

This could also be linked to #976, in that the problem is that params that don't have a default value are handled as always being different to the initialised config param.

ewels added a commit to ewels/nf-core-tools that referenced this issue Mar 26, 2021
* Don't set defaults to empty strings if no default in the schema
* Fix logic for printing a param to the summary if there is no default
* Rewrite the colour for the summary log
* Other minor tidying

Fixes nf-core#971
@ewels ewels closed this as completed Mar 30, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority schema template nf-core pipeline/component template
Projects
None yet
Development

No branches or pull requests

1 participant