nextflow-io / nextflow

A DSL for data-driven computational pipelines
http://nextflow.io
Apache License 2.0
2.71k stars 620 forks source link

Unexpected behavior of `ParamsMap.copyWith()` #2084

Closed yqshao closed 2 years ago

yqshao commented 3 years ago

Bug report

ParamsMap.put updates camal case name and hypen name simultaneously, but ParamsMap.copyWith() does not work with a Map containing only camal case keys.

Expected behavior and actual behavior

Expected: the method returns a copy with keys overwritten according to a map. Actual: keys only in camel case not are overwritten (unless both Camel and Hypen case are specified)

Steps to reproduce the problem

With this minimal script:

#!/usr/bin/env nextflow
params.foo = 100
params.fooBar = 100
println params.copyWith([foo:0, fooBar:0])
// will work with:
// params.copyWith([foo:0, 'foo-bar': 0, 'fooBar':0), or
// params.copyWith([foo:0, 'fooBar': 0] as ParamsMap)

Program output

N E X T F L O W  ~  version 21.04.0
Launching `minimal.nf` [elated_liskov] - revision: fbf82ffb54
[foo:0, fooBar:100, foo-bar:100]

Environment

Additional context

I'd like to attach what I'm trying to achieve, as I'm not quite sure whether this usage of params.copyWith() is intended in Nextflow.

#!/usr/bin/env nextflow
nextflow.enable.dsl = 2
params.foo = 100
params.fooBar = 100

workflow subflow {
    take: inputs
    main:
    setups = inputs.map {params.copyWith(it)}
    setups.view() // do something with the setup
}

workflow {
    subflow(Channel.of([:]))
}

Here subflow is something, to be specific, a molecular dynamics simulation given a few parameters. I would like it to be resued as a sub-workflow in more complex workflows where its parameters are adjusted. The current setup presumably would allow assigning one parameter through CLI option and do a grid scan for another.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ahstram commented 1 year ago

@yqshao , I'm not sure how helpful this is, but the logic for allowing params to be overwritten is here:

https://github.com/nextflow-io/nextflow/blob/bc8700a8ebfc15903ed99ab2fd4066e2b2d0d275/modules/nextflow/src/main/groovy/nextflow/script/ScriptBinding.groovy#L288-L298

yqshao commented 1 year ago

@ahstram

Thanks for looking into this! It has been a while ago; and the usage seems rare enough, so I decide to not rely on it in my project. Anyways, I think you are right about the source of the issue, more precisely, I think it's also related to the logics to initialize a ParamsMap with an override here: https://github.com/nextflow-io/nextflow/blob/bc8700a8ebfc15903ed99ab2fd4066e2b2d0d275/modules/nextflow/src/main/groovy/nextflow/script/ScriptBinding.groovy#L234-L242

The readOnlyNames must be cleared before overriding, and the override will be silently ignored if there are two names (in the case of Camel or hyphen-case keys). So I suppose a solution is to clear both keys in the allowNames method. Not sure if it's worth a PR though.