Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
### Linting

- accept `process_low_memory` as a standard module label ([#4264](https://github.com/nf-core/tools/pull/4264))
- improve linting for `modules.json` to add support for only installing subworkflows from a repository and provide more explicit error messages ([#4287](https://github.com/nf-core/tools/pull/4287))

### Modules

Expand Down
52 changes: 26 additions & 26 deletions nf_core/pipelines/lint/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,32 +25,32 @@ def modules_json(self) -> dict[str, list[str]]:
all_modules_passed = True

for repo in modules_json_dict["repos"]:
# Check if the modules.json has been updated to keep the
if "modules" not in modules_json_dict["repos"][repo] or not repo.startswith("http"):
failed.append(
"Your `modules.json` file is outdated. "
"It will be automatically generated by running any module command."
)
continue

for install_dir in modules_json_dict["repos"][repo]["modules"]:
for module, module_entry in modules_json_dict["repos"][repo]["modules"][install_dir].items():
if not Path(modules_dir, install_dir, module).exists():
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` found in `modules.json` but module is not installed in "
"pipeline."
)
all_modules_passed = False
if module_entry.get("branch") is None:
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` is missing branch information."
)
if module_entry.get("git_sha") is None:
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` is missing version information."
)
if all_modules_passed:
passed.append("Only installed modules found in `modules.json`")
if not repo.startswith("http"):
failed.append(f'Repository link for {repo} doesn\'t start with "http" in `modules.json`.')

if "modules" in modules_json_dict["repos"][repo]:
# Module linting
for install_dir in modules_json_dict["repos"][repo]["modules"]:
for module, module_entry in modules_json_dict["repos"][repo]["modules"][install_dir].items():
if not Path(modules_dir, install_dir, module).exists():
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` found in `modules.json` but module is not installed in "
"pipeline."
)
all_modules_passed = False
if module_entry.get("branch") is None:
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` is missing branch information."
)
if module_entry.get("git_sha") is None:
failed.append(
f"Entry for `{Path(modules_dir, install_dir, module)}` is missing version information."
)
if all_modules_passed:
passed.append("Only installed modules found in `modules.json`")

if not any(component in modules_json_dict["repos"][repo] for component in ["modules", "subworkflows"]):
failed.append(f"No modules or subworkflows installed for {repo} in `modules.json`.")
else:
warned.append("Could not open `modules.json` file.")

Expand Down
35 changes: 35 additions & 0 deletions tests/pipelines/lint/test_modules_json.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,45 @@
import json

from nf_core.modules.modules_json import (
ModulesJson,
ModulesJsonType,
Comment thread
mashehu marked this conversation as resolved.
)

from ..test_lint import TestLint


class TestLintModulesJson(TestLint):
def setUp(self) -> None:
super().setUp()
self._modules_json_object: ModulesJson = ModulesJson(self.pipeline_dir)
self._modules_json_object.load()
self.modules_json_path = self._modules_json_object.modules_json_path
self.modules_json: ModulesJsonType | None = self._modules_json_object.modules_json
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.

This attribute can't actually be None due to .load() on line 15, but mypy throws an error since it is inherited from ModulesJson.modules_json which has ModulesJsonType | None typehint.

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.

you can just throw in an assert to make mypy happy


def _update_modules_json(self, new_modules_json) -> None:
# Update modules_json so that it is concordant at all levels of the class
self.modules_json = new_modules_json
self._modules_json_object.modules_json = new_modules_json
with open(self.modules_json_path, "w") as modules_json_file:
json.dump(new_modules_json, modules_json_file)

def test_modules_json_pass(self):
self.lint_obj._load()
results = self.lint_obj.modules_json()
assert len(results.get("warned", [])) == 0
assert len(results.get("failed", [])) == 0
assert len(results.get("passed", [])) > 0

def test_modules_json_with_only_subworkflow_entries(self):
# Create a new_modules_json with only "subworkflows" component for all repos
new_modules_json = self.modules_json
for repo in new_modules_json["repos"]:
new_modules_json["repos"][repo] = {"subworkflows": new_modules_json["repos"][repo]["subworkflows"]}

self._update_modules_json(new_modules_json)

# Check linting results
results = self.lint_obj.modules_json()
assert len(results.get("warned", [])) == 0
assert len(results.get("failed", [])) == 0
assert len(results.get("passed", [])) == 0
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.

With the current set up there is no subworkflow linting so no passed is returned. Should we open an issue to add subworkflow linting in the future?

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.

yes, please do that.

Loading