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

Improve process label linting #2227

Merged
merged 10 commits into from
Apr 26, 2023
73 changes: 53 additions & 20 deletions nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,26 +235,9 @@ def check_process_section(self, lines, fix_version, progress_bar):
self.failed.append(("process_capitals", "Process name is not in capital letters", self.main_nf))

# Check that process labels are correct
Aratz marked this conversation as resolved.
Show resolved Hide resolved
correct_process_labels = ["process_single", "process_low", "process_medium", "process_high", "process_long"]
process_label = [l for l in lines if l.lstrip().startswith("label")]
if len(process_label) > 0:
try:
process_label = re.search("process_[A-Za-z]+", process_label[0]).group(0)
except AttributeError:
process_label = re.search("'([A-Za-z_-]+)'", process_label[0]).group(0)
finally:
if not process_label in correct_process_labels:
self.warned.append(
(
"process_standard_label",
f"Process label ({process_label}) is not among standard labels: `{'`,`'.join(correct_process_labels)}`",
self.main_nf,
)
)
else:
self.passed.append(("process_standard_label", "Correct process label", self.main_nf))
else:
self.warned.append(("process_standard_label", "Process label unspecified", self.main_nf))
check_process_labels(self, lines)

# Deprecated enable_conda
for i, l in enumerate(lines):
url = None
l = l.strip(" '\"")
Expand Down Expand Up @@ -409,6 +392,56 @@ def check_process_section(self, lines, fix_version, progress_bar):
return docker_tag == singularity_tag


def check_process_labels(self, lines):
correct_process_labels = ["process_single", "process_low", "process_medium", "process_high", "process_long"]
all_labels = [l.strip() for l in lines if l.lstrip().startswith("label ")]
bad_labels = []
good_labels = []
if len(all_labels) > 0:
for label in all_labels:
try:
label = re.match("^label\s+([a-zA-Z0-9_-]+)$", label).group(1)
except AttributeError:
self.warned.append(
(
"process_standard_label",
f"Specified label appears to contain non-alphanumerics: {label}",
self.main_nf,
)
)
continue
if label not in correct_process_labels:
bad_labels.append(label)
else:
good_labels.append(label)
if len(good_labels) > 1:
self.warned.append(
(
"process_standard_label",
f"Conflicting process labels found: `{'`,`'.join(good_labels)}`",
self.main_nf,
)
)
elif len(good_labels) == 1:
self.passed.append(("process_standard_label", "Correct process label", self.main_nf))
else:
self.warned.append(("process_standard_label", "Standard process label not found", self.main_nf))
if len(bad_labels) > 0:
self.warned.append(
("process_standard_label", f"Non-standard labels found: `{'`,`'.join(bad_labels)}`", self.main_nf)
)
if len(all_labels) > len(set(all_labels)):
self.warned.append(
(
"process_standard_label",
f"Duplicate labels found: `{'`,`'.join(sorted(all_labels))}`",
self.main_nf,
)
)
else:
self.warned.append(("process_standard_label", "Process label not specified", self.main_nf))


def _parse_input(self, line_raw):
"""
Return list of input channel names from an input line.
Expand Down
111 changes: 111 additions & 0 deletions tests/modules/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest

import nf_core.modules
from nf_core.modules.lint import main_nf

from ..utils import GITLAB_URL, set_wd
from .patch import BISMARK_ALIGN, CORRECT_SHA, PATCH_BRANCH, REPO_NAME, modify_main_nf
Expand Down Expand Up @@ -105,3 +106,113 @@ def test_modules_lint_patched_modules(self):
assert len(module_lint.failed) == 1
assert len(module_lint.passed) > 0
assert len(module_lint.warned) >= 0


# A skeleton object with the passed/warned/failed list attrs
# Use this in place of a ModuleLint object to test behaviour of
# linting methods which don't need the full setup
class MockModuleLint:
def __init__(self):
self.passed = []
self.warned = []
self.failed = []

self.main_nf = "main_nf"


PROCESS_LABEL_GOOD = (
"""
label process_high
cpus 12
""",
1,
0,
0,
)
PROCESS_LABEL_NON_ALPHANUMERIC = (
"""
label a:label:with:colons
cpus 12
""",
0,
2,
0,
)
PROCESS_LABEL_GOOD_CONFLICTING = (
"""
label process_high
label process_low
cpus 12
""",
0,
1,
0,
)
PROCESS_LABEL_GOOD_DUPLICATES = (
"""
label process_high
label process_high
cpus 12
""",
0,
2,
0,
)
PROCESS_LABEL_GOOD_AND_NONSTANDARD = (
"""
label process_high
label process_extra_label
cpus 12
""",
1,
1,
0,
)
PROCESS_LABEL_NONSTANDARD = (
"""
label process_extra_label
cpus 12
""",
0,
2,
0,
)
PROCESS_LABEL_NONSTANDARD_DUPLICATES = (
"""
label process_extra_label
label process_extra_label
cpus 12
""",
0,
3,
0,
)
PROCESS_LABEL_NONE_FOUND = (
"""
cpus 12
""",
0,
1,
0,
)

PROCESS_LABEL_TEST_CASES = [
PROCESS_LABEL_GOOD,
PROCESS_LABEL_NON_ALPHANUMERIC,
PROCESS_LABEL_GOOD_CONFLICTING,
PROCESS_LABEL_GOOD_DUPLICATES,
PROCESS_LABEL_GOOD_AND_NONSTANDARD,
PROCESS_LABEL_NONSTANDARD,
PROCESS_LABEL_NONSTANDARD_DUPLICATES,
PROCESS_LABEL_NONE_FOUND,
]


def test_modules_lint_check_process_labels(self):
for test_case in PROCESS_LABEL_TEST_CASES:
process, passed, warned, failed = test_case
mocked_ModuleLint = MockModuleLint()
main_nf.check_process_labels(mocked_ModuleLint, process.splitlines())
assert len(mocked_ModuleLint.passed) == passed
assert len(mocked_ModuleLint.warned) == warned
assert len(mocked_ModuleLint.failed) == failed
1 change: 1 addition & 0 deletions tests/test_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ def test_modulesrepo_class(self):
test_modules_install_trimgalore_twice,
)
from .modules.lint import (
test_modules_lint_check_process_labels,
test_modules_lint_empty,
test_modules_lint_gitlab_modules,
test_modules_lint_multiple_remotes,
Expand Down