Skip to content

Commit

Permalink
Merge pull request #921 from KevinMenden/patch-fix-triage-3
Browse files Browse the repository at this point in the history 
Patch fix triage 3
  • Loading branch information
@ewels
ewels authored Mar 19, 2021
2 parents 5e032b4 + 0d5ba18 commit e567a63
Showing 1 changed file with 53 additions and 43 deletions.
96 changes: 53 additions & 43 deletions nf_core/modules/lint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from __future__ import print_function
import logging
import operator
import os
import questionary
import re
Expand All @@ -34,6 +35,17 @@ class ModuleLintException(Exception):
pass


class LintResult(object):
""" An object to hold the results of a lint test """

def __init__(self, mod, lint_test, message, file_path):
self.mod = mod
self.lint_test = lint_test
self.message = message
self.file_path = file_path
self.module_name = mod.module_name


class ModuleLint(object):
"""
An object for linting modules either in a clone of the 'nf-core/modules'
Expand Down Expand Up @@ -144,8 +156,8 @@ def lint_local_modules(self, local_modules):
mod_object.main_nf = mod
mod_object.module_name = os.path.basename(mod)
mod_object.lint_main_nf()
self.passed = [(mod_object, m) for m in mod_object.passed]
self.warned = [(mod_object, m) for m in mod_object.warned + mod_object.failed]
self.passed += [LintResult(mod_object, m[0], m[1], m[2]) for m in mod_object.passed]
self.warned += [LintResult(mod_object, m[0], m[1], m[2]) for m in mod_object.warned + mod_object.failed]

def lint_nfcore_modules(self, nfcore_modules):
"""
Expand Down Expand Up @@ -174,12 +186,9 @@ def lint_nfcore_modules(self, nfcore_modules):
for mod in nfcore_modules:
progress_bar.update(lint_progress, advance=1, test_name=mod.module_name)
passed, warned, failed = mod.lint()
passed = [(mod, m) for m in passed]
warned = [(mod, m) for m in warned]
failed = [(mod, m) for m in failed]
self.passed += passed
self.warned += warned
self.failed += failed
self.passed += [LintResult(mod, m[0], m[1], m[2]) for m in passed]
self.warned += [LintResult(mod, m[0], m[1], m[2]) for m in warned]
self.failed += [LintResult(mod, m[0], m[1], m[2]) for m in failed]

def get_repo_type(self):
"""
Expand Down Expand Up @@ -265,12 +274,18 @@ def _print_results(self, show_passed=False):

log.debug("Printing final results")
console = Console(force_terminal=rich_force_colors())

# Sort the results
self.passed.sort(key=operator.attrgetter("message", "module_name"))
self.warned.sort(key=operator.attrgetter("message", "module_name"))
self.failed.sort(key=operator.attrgetter("message", "module_name"))

# Find maximum module name length
max_mod_name_len = 40
for idx, tests in enumerate([self.passed, self.warned, self.failed]):
try:
for mod, msg in tests:
max_mod_name_len = max(len(mod.module_name), max_mod_name_len)
for lint_result in tests:
max_mod_name_len = max(len(lint_result.module_name), max_mod_name_len)
except:
pass

Expand All @@ -284,17 +299,17 @@ def format_result(test_results, table):
# I'd like to make an issue about this on the rich repo so leaving here in case there is a future fix
last_modname = False
row_style = None
for mod, result in test_results:
if last_modname and mod.module_name != last_modname:
for lint_result in test_results:
if last_modname and lint_result.module_name != last_modname:
if row_style:
row_style = None
else:
row_style = "magenta"
last_modname = mod.module_name
last_modname = lint_result.module_name
table.add_row(
Markdown(f"{mod.module_name}"),
os.path.relpath(result[2], self.dir),
Markdown(f"{result[1]}"),
Markdown(f"{lint_result.module_name}"),
Markdown(f"{lint_result.file_path}"),
os.path.relpath(lint_result.message, self.dir),
style=row_style,
)
return table
Expand Down Expand Up @@ -393,13 +408,11 @@ def check_module_changes(self, nfcore_modules):

if r.status_code != 200:
self.warned.append(
(
LintResult(
mod,
(
"check_local_copy",
f"Could not fetch remote copy, skipping comparison.",
f"{os.path.join(mod.module_dir, f)}",
),
"check_local_copy",
f"Could not fetch remote copy, skipping comparison.",
f"{os.path.join(mod.module_dir, f)}",
)
)
else:
Expand All @@ -409,24 +422,20 @@ def check_module_changes(self, nfcore_modules):
if local_copy != remote_copy:
all_modules_up_to_date = False
self.warned.append(
(
LintResult(
mod,
(
"check_local_copy",
"Local copy of module outdated",
f"{os.path.join(mod.module_dir, f)}",
),
"check_local_copy",
"Local copy of module outdated",
f"{os.path.join(mod.module_dir, f)}",
)
)
except UnicodeDecodeError as e:
self.warned.append(
(
LintResult(
mod,
(
"check_local_copy",
f"Could not decode file from {url}. Skipping comparison ({e})",
f"{os.path.join(mod.module_dir, f)}",
),
"check_local_copy",
f"Could not decode file from {url}. Skipping comparison ({e})",
f"{os.path.join(mod.module_dir, f)}",
)
)

Expand Down Expand Up @@ -513,7 +522,8 @@ def lint_module_tests(self):

def lint_meta_yml(self):
""" Lint a meta yml file """
required_keys = ["input", "output"]
required_keys = ["name", "input", "output"]
required_keys_lists = ["intput", "output"]
try:
with open(self.meta_yml, "r") as fh:
meta_yaml = yaml.safe_load(fh)
Expand All @@ -529,7 +539,7 @@ def lint_meta_yml(self):
if not rk in meta_yaml.keys():
self.failed.append(("meta_required_keys", f"`{rk}` not specified", self.meta_yml))
contains_required_keys = False
elif not isinstance(meta_yaml[rk], list):
elif not isinstance(meta_yaml[rk], list) and rk in required_keys_lists:
self.failed.append(("meta_required_keys", f"`{rk}` is not a list", self.meta_yml))
all_list_children = False
if contains_required_keys:
Expand All @@ -551,13 +561,13 @@ def lint_meta_yml(self):
else:
self.failed.append(("meta_output", "`{output}` missing in `meta.yml`", self.meta_yml))

# confirm that the name matches the process name in main.nf
if meta_yaml["name"].upper() == self.process_name:
self.passed.append(("meta_name", "Correct name specified in `meta.yml`", self.meta_yml))
else:
self.failed.append(
("meta_name", "Conflicting process name between `meta.yml` and `main.nf`", self.meta_yml)
)
# confirm that the name matches the process name in main.nf
if meta_yaml["name"].upper() == self.process_name:
self.passed.append(("meta_name", "Correct name specified in `meta.yml`", self.meta_yml))
else:
self.failed.append(
("meta_name", "Conflicting process name between `meta.yml` and `main.nf`", self.meta_yml)
)

def lint_main_nf(self):
"""
Expand Down

0 comments on commit e567a63

Please sign in to comment.