Skip to content

Convert to typed params - #141

Open
itrujnara wants to merge 9 commits into
nf-core:devfrom
itrujnara:typed_params
Open

Convert to typed params#141
itrujnara wants to merge 9 commits into
nf-core:devfrom
itrujnara:typed_params

Conversation

@itrujnara

@itrujnara itrujnara commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Convert the pipeline to strict syntax typed params

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • CHANGELOG.md is updated.

@github-actions

github-actions Bot commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Posted for pipeline commit 2740365

+| ✅ 199 tests passed       |+
#| ❔   5 tests were ignored |#
#| ❔   1 tests had warnings |#
!| ❗   6 tests had warnings |!
Details

❗ Test warnings:

  • files_exist - File not found: conf/igenomes_ignored.config
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in CONTRIBUTING.md: Add any pipeline specific contribution guidelines here, such as coding styles, procedures, checklists etc.

❔ Tests ignored:

❔ Tests fixed:

✅ Tests passed:

Run details

  • nf-core/tools version 4.0.3
  • Run at 2026-07-28 13:09:09

@github-actions

github-actions Bot commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown

❌ nf-test failed with latest Nextflow version

Note

Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
Please check if the failure is expected with newer (edge-)releases of Nextflow or if it needs fixing.

  • ❌ docker | latest-everything | Shard 4/7

See the full run for details.

dialvarezs
dialvarezs approved these changes Jul 28, 2026
View reviewed changes

@dialvarezs dialvarezs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks neat!
I'm going to use as a reference when I convert mag to typed params.

My main question is why some params are in both places, and also some minor type correctness.

Comment thread main.nf
Comment on lines +65 to +66
min_identity: Integer = 90
min_coverage: Integer = 80

@dialvarezs dialvarezs Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't these be Float instead?

Comment thread nextflow.config

// ID merging options
skip_merge = false
min_identity = 90

@dialvarezs dialvarezs Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any reason why these params need to be kept here?

Comment thread main.nf
email_on_fail: String? = null
plaintext_email: Boolean = false
monochrome_logs: Boolean = false
help: Boolean = false

@dialvarezs dialvarezs Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

help can be String too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dialvarezs dialvarezs dialvarezs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@itrujnara @dialvarezs