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

Document ext.args in meta yml #3451

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

Conversation

awgymer
Copy link
Contributor

@awgymer awgymer commented Feb 12, 2025

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

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
Copy link

codecov bot commented Feb 12, 2025

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

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 ⚠️
Additional details and impacted files

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

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.

nf_core/modules/lint/meta_yml.py Show resolved Hide resolved
nf_core/components/nfcore_component.py Show resolved Hide resolved
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.

Add args section in meta.yml
2 participants