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

Fix meta_yml linting error #3317

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

Conversation

lmReef
Copy link

@lmReef lmReef commented Dec 3, 2024

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated

Description

This PR is to fix an issue where running nf-core modules lint -a -k meta_yml always has failing tests due to module.process_name always being "" in nfcore_component.py.

It gets initialized to "" then eventually gets updated in main_nf but not in any others which results in the failed meta_yml tests unless -k main_nf is specified as well

I've added a simple function for the component to set it's process_name itself rather than it being set in other separate parts of the code. It uses the same regex expression that main_nf.py uses

@lmReef lmReef marked this pull request as ready for review December 3, 2024 21:31
@lmReef
Copy link
Author

lmReef commented Dec 3, 2024

I think the issue was added here when the test was added back, but it was not so obvious that module.process_name can sometimes still be unset in that context

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.

LGTM, tests are currently failing due to a Java version, a fix is on the way, we can merge once that is fixed 👍
Thanks for this PR!

@@ -96,6 +95,9 @@ def __init__(
self.test_yml = None
self.test_main_nf = None

# Set process_name after self.main_nf is defined
self.process_name = self._get_process_name()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.process_name = self._get_process_name()
self.process_name: String = self._get_process_name()

Copy link
Author

Choose a reason for hiding this comment

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

Thanks I missed that! added.

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.

2 participants