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

Conceptual error in process selector documentation #4416

Closed
wants to merge 5 commits into from

Conversation

saulpierotti
Copy link

Hi, the documentation for this seems inaccurate to me. While the first part is true that the withName selector applies also to the original name of a module, not only the import name, in my usage experience it does also apply to the import name. For example if I have a process named FOO and I import it with import FOO as BAR, a withName:BAR selector will work, and also a withName:FOO selector will. The documentation suggests that only withName:FOO works, which I believe to be incorrect.

Hi, the documentation for this seems inaccurate to me. While the first part is true that the withName selector applies also to the original name of a module, not only the import name, in my usage experience it does also apply to the import name. For example if I have a process named FOO and I import it with import FOO as BAR, a withName:BAR selector will work, and also a withName:FOO selector will. The documentation suggests that only withName:FOO works, which I believe to be incorrect.

Signed-off-by: Saul Pierotti <[email protected]>
@netlify
Copy link

netlify bot commented Oct 16, 2023

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit 41c3cc4
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/659f6ebac7d9240008f0f647
😎 Deploy Preview https://deploy-preview-4416--nextflow-docs-staging.netlify.app/config
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bentsherman
Copy link
Member

If this is the case then we should probably change Nextflow to not apply process selectors for the base name to aliases.

@saulpierotti
Copy link
Author

saulpierotti commented Oct 17, 2023

I actually find it useful to have it this way - I can have the same module and import it with two different aliases, and then have it run with different parameters/modes according to the alias. An example is https://nf-co.re/modules/stitch, which I import once for generating the input only with certain ext.args, and once for actually running the imputation with other ext.args.

Or for example a very general module like the plink2 module. I may want to use it several times for doing different things and manage it via different ext.args.

@mribeirodantas
Copy link
Member

It seems to be the case, I can reproduce on my side, @bentsherman.

.
├── main.nf
├── modules
│   └── foo
│       └── main.nf
└── nextflow.config

3 directories, 3 files

./main.nf

include { FOO        } from './modules/foo/main.nf'
include { FOO as BAR } from './modules/foo/main.nf'

workflow {
  FOO()
  BAR()
}

nextflow.config

process {
  withName: FOO {
    cpus = 1
  }
  withName: BAR {
    cpus = 2
  }
}

./modules/foo/main.nf

process FOO {
  debug true

  output:
  stdout

  script:
  """
  echo ${task.cpus}
  """
}

Output:

N E X T F L O W  ~  version 23.10.0
Launching `main.nf` [trusting_colden] DSL2 - revision: 466eb880a9
executor >  local (2)
[9f/cf382e] process > FOO [100%] 1 of 1 ✔
[0e/748233] process > BAR [100%] 1 of 1 ✔
1

2

docs/config.md Outdated Show resolved Hide resolved
mribeirodantas and others added 2 commits October 18, 2023 10:10
Co-authored-by: Marcel Ribeiro-Dantas <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
Copy link
Member

@bentsherman bentsherman left a comment

Choose a reason for hiding this comment

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

Fair enough, I guess if you don't want the base config to be propagated to aliases then you can simply not use it and alias every process inclusion.

@bentsherman
Copy link
Member

cc @marcodelapierre

consider folding this PR into the module config #4510

@marcodelapierre marcodelapierre mentioned this pull request Jan 11, 2024
@bentsherman
Copy link
Member

Closing in favor of #4510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants