Skip to content

Allow task.ext.prefix2 in modules linting#4234

Merged
SPPearce merged 9 commits into
devfrom
prefix2
Apr 29, 2026
Merged

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

Conversation

@SPPearce

@SPPearce SPPearce commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

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

@maxulysse maxulysse 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.

LGTM

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

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

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

@mahesh-panchal mahesh-panchal 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.

The invalid keys are not in the test itself

@mahesh-panchal mahesh-panchal 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.

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