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

Template: remove try-catch blocks from nextflow.config #3167

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

mirpedrol
Copy link
Member

Close #3132

@mirpedrol
Copy link
Member Author

One problem with this syntax is that the first time a user creates a pipeline, this will fail as nf-core/configs does not contain the pipeline config.
It can be solved by providing a false params.custom_config_base but it's not ideal. I would consider commenting out these lines on the template again and let users decide if they want to include a pipeline specific config.

System.err.println("WARNING: Could not load nf-core/config/{{ short_name }} profiles: ${params.custom_config_base}/pipeline/{{ short_name }}.config")
}
// TODO nf-core: Optionally, you can add a pipeline-specific nf-core config at https://github.com/nf-core/configs
includeConfig !System.getenv('NXF_OFFLINE') && params.custom_config_base ? "${params.custom_config_base}/pipeline/{{ short_name }}.config" : "/dev/null"
Copy link
Member

Choose a reason for hiding this comment

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

I would consider commenting out these lines on the template again and let users decide if they want to include a pipeline specific config.

Agree 👍🏻

Suggested change
includeConfig !System.getenv('NXF_OFFLINE') && params.custom_config_base ? "${params.custom_config_base}/pipeline/{{ short_name }}.config" : "/dev/null"
// includeConfig !System.getenv('NXF_OFFLINE') && params.custom_config_base ? "${params.custom_config_base}/pipeline/{{ short_name }}.config" : "/dev/null"

Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that a pipeline developer has to do this?

If so I don't like that idea so much as it never gets uncommented (as much people don't know about it), but then you have users come along asking for the support which then requires a whole new release of the pipeline.

We specifically added automatically generated the 'dummy' configs to nf-core/configs recently so it's a seamless experience. A pipeline developer just needs to wait overnight for the dummy configs to be added (with the caveat the pipeline has to be under the nf-core organisation).

The other solution I can think to get around this: is to comment it out, but make a release linting error that it must be uncommented before release. Thus from first release it's supported

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem now is that without the try/except it will fail right after you use nf-core pipelines create, because the config is not added yet to nf-core.
The release linting error sounds like a good solution to me :)

@ewels ewels added this to the 3.0 milestone Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants