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

DSL2+ #312

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

DSL2+ #312

wants to merge 26 commits into from

Conversation

bentsherman
Copy link

@bentsherman bentsherman commented May 21, 2024

This PR refactors the fetchngs pipeline to use some of the newest features in Nextflow and follow some recommended practices. All of these changes should be possible with Nextflow 24.10.

  • Use strict syntax. Currently enforced by the language server, will be enforced by Nextflow in the future. Some of these changes might need to be applied to the upstream pipeline template.

  • Use params in top-level workflow. Pass params into processes and workflows as explicit inputs.

  • Use eval output, topic channels to collect tool versions. Simplify the collection of tool versions, removes lots of boilerplate from processes and workflows.

  • Replace publishDir with workflow outputs. Move publish definition to workflow level by publishing channels. Use the index file feature to generate the samplesheet.

@bentsherman bentsherman marked this pull request as ready for review May 21, 2024 14:53
This was referenced May 21, 2024
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Author

Closing in favor of #309

  • Eval outputs and topic channels are being implemented in modules, see Use eval to collect version modules#5834
  • The nf-core pipeline template was updated in v3 to comply with strict syntax, which is also new defined in the docs and enforced by the language server
  • Parameter schema is now used by the language server, can be incorporated into Nextflow with less urgency, pending further design

Based on discussions with the community, I have concluded that the other proposed changes (use params only in entry workflow, remove ext config, workflow outputs) will be much easier to do with static types, so I'm not going to push for it very much in the meantime. IMO it makes more sense to wait for static types and refactor your pipeline once, rather than refactor now with suboptimal syntax and then refactor again in a year, which wouldn't bring much benefit, especially now with the language server.

I do encourage fetchngs to go ahead and update where appropriate, but we can pursue these changes in smaller pieces, in particular:

  • comply with strict syntax (mostly covered by template v3)
  • topic channels, eval outputs (once modules are updated)
  • workflow outputs

@bentsherman bentsherman closed this Nov 3, 2024
@bentsherman bentsherman deleted the dsl2-plus branch November 3, 2024 19:49
@bentsherman bentsherman restored the dsl2-plus branch December 4, 2024 23:41
@bentsherman bentsherman reopened this Dec 4, 2024
Copy link

github-actions bot commented Dec 5, 2024

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 08a3361

+| ✅ 167 tests passed       |+
#| ❔  12 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-12-10 16:59:35

@bentsherman bentsherman changed the title Proposal: DSL2+ DSL2+ Dec 5, 2024
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Author

I have revived this PR and rescoped it to what we can do now with Nextflow 24.10 -- see the updated PR description. Let me know what you guys think.

Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
main.nf Outdated
Comment on lines 106 to 110
def dirs = [
'fastq': 'fastq',
'md5': 'fastq/md5'
]
return { file -> "${dirs[file.extension]}/${file.name}" }
Copy link
Author

Choose a reason for hiding this comment

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

A more explicit way to write this could be as a list of source-target pairs:

def files = [
    sample.fastq_1,
    sample.fastq_2,
    sample.md5_1,
    sample.md5_2
]
def dirs = [
    'fastq': 'fastq',
    'md5': 'fastq/md5'
]
return files
    .findAll { file -> file != null }
    .collect { file ->
        [ file, "${dirs[file.extension]}/${file.name}" ]
    }

It's more verbose but it spells out more clearly what it going on. But as long as Nextflow can infer the files list automatically, the user should only need to return the mapping closure

Signed-off-by: Ben Sherman <[email protected]>
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.

1 participant