+
Skip to content

Document ext.args in meta yml #3451

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Document ext.args in meta yml #3451

wants to merge 5 commits into from

Conversation

awgymer
Copy link
Contributor

@awgymer awgymer commented Feb 12, 2025 •
edited
Loading

This closes #3444

An analogous PR to nf-core/modules will need to be made to update the module meta yml schema json if this is approved.

This PR adds:

  • ext.args info from meta.yml displayed by modules info
  • Lint check which warns if any expected ext.args* are not documented in meta.yml

Will need to add a test for this lint check/info output but not sure how best to achieve this before an update to nf-core/modules?

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

@github-actions GitHub Actions
Copy link
Contributor

This PR is against the main branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @awgymer,

It looks like this pull-request is has been made against the awgymer/nf-core-tools main branch.
The main branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to main are only allowed if they come from the awgymer/nf-core-tools dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@awgymer awgymer requested a review from mirpedrol February 12, 2025 10:25
@awgymer awgymer changed the base branch from main to dev February 12, 2025 10:25
@codecov Codecov
Copy link

codecov bot commented Feb 12, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 69.56522% with 14 lines in your changes missing coverage. Please review.

Project coverage is 76.48%. Comparing base (abb3acf) to head (ae37baa).
Report is 765 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/components/info.py 20.00% 8 Missing ⚠️
nf_core/components/nfcore_component.py 73.68% 5 Missing ⚠️
nf_core/modules/lint/meta_yml.py 94.11% 1 Missing ⚠️

☔ 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.

mirpedrol
mirpedrol reviewed Feb 12, 2025
View reviewed changes
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

Looking good!
We could automatically add all the args to the meta.yml when running nf-core modules lint --fix, this is done by update_meta_yml_file() function in nf_core/modules/lint/__init__.py. Then we can gradually keep adding the change to modules, so we don't have to do a big bulk update PR, especially since this will be a warning, not an error.
I am also working on some modifications to the meta.yml here, these shouldn't affect your changes, but it would be good if we can avoid big merge conflicts.

@arnaudbore
Copy link

Thumbs up for this PR 👍

@arnaudbore
Copy link

Any update @mirpedrol, @awgymer ?

@mirpedrol mirpedrol mentioned this pull request Mar 12, 2025
Merged
@mirpedrol
Copy link
Member

Hello, I have opened the PR to modules for adding this to the JSON file nf-core/modules#7783

@mirpedrol
Copy link
Member

Hi @awgymer, I merged the PR on modules, this should be updated to use an array for args. Will you have time to work on this or do you need someone to take over? Thanks!
We should also add a test for this 😉

@awgymer
Copy link
Contributor Author

awgymer commented Jun 19, 2025

I am on holiday at the moment so I wouldn't be working on it for at least another week.

If you want to update the PR and merge it before that please feel free 🥹

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

@mirpedrol mirpedrol mirpedrol left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add args section in meta.yml
3 participants
@awgymer @arnaudbore @mirpedrol
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载