Skip to content

Allow task.ext.prefix2 in modules linting#4234

Merged
SPPearce merged 9 commits intodevfrom
prefix2
Apr 29, 2026
Merged

Allow task.ext.prefix2 in modules linting#4234
SPPearce merged 9 commits intodevfrom
prefix2

Conversation

@SPPearce
Copy link
Copy Markdown
Contributor

@SPPearce SPPearce commented Apr 28, 2026

In the April maintainers meeting we agreed to allow task.ext.prefix2 as an option for modules that really need it, as an analogy to ext.args, ext.args2 etc.
This PR actually allows any number of numbered prefix values, in the same way as we do for args (using the same code).

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Copy link
Copy Markdown
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

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

LGTM

@SPPearce SPPearce linked an issue Apr 28, 2026 that may be closed by this pull request
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.39%. Comparing base (93221e4) to head (8a653c3).
⚠️ Report is 10 commits behind head on dev.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread tests/modules/lint/test_main_nf.py Outdated
)
assert len(mock_lint.failed) == 0

# ext.prefixN where N >= 2 should be valid
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought only prefix2 would be allowed, not prefix3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At the moment I just copied the args code and added prefix as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Swapped to just adding prefix2 directly to the allowed keys.

Comment thread tests/modules/lint/test_main_nf.py Outdated
mock_lint,
[
"""
def prefix2 = task.ext.prefix2 ?: ''
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.

Can you add a test for prefix1 and other not permitted combos too please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a test for prefix1.

Comment thread tests/modules/lint/test_main_nf.py
Comment thread tests/modules/lint/test_main_nf.py
Copy link
Copy Markdown
Member

@mahesh-panchal mahesh-panchal left a comment

Choose a reason for hiding this comment

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

The invalid keys are not in the test itself

Copy link
Copy Markdown
Member

@mahesh-panchal mahesh-panchal left a comment

Choose a reason for hiding this comment

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

Thank you. Don't forget the website docs too.

@SPPearce
Copy link
Copy Markdown
Contributor Author

nf-core/website#4182

@SPPearce SPPearce merged commit 7071315 into dev Apr 29, 2026
118 checks passed
@SPPearce SPPearce deleted the prefix2 branch April 29, 2026 08:31
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.

Allow ext.prefix2

5 participants