From 42e59285a13252d98d312bed14c8b7f8cae93576 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 19 Mar 2021 11:20:24 +0100 Subject: [PATCH 1/5] fixed missing key bug in meta.yml --- nf_core/modules/lint.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py index 00ef496e9d..8212a77a99 100644 --- a/nf_core/modules/lint.py +++ b/nf_core/modules/lint.py @@ -510,7 +510,7 @@ def lint_module_tests(self): def lint_meta_yml(self): """ Lint a meta yml file """ - required_keys = ["input", "output"] + required_keys = ["name", "input", "output"] try: with open(self.meta_yml, "r") as fh: meta_yaml = yaml.safe_load(fh) @@ -548,13 +548,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): """ From 86e2560bf6db2f08426037876ee4f829e0724cfc Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 19 Mar 2021 11:38:47 +0100 Subject: [PATCH 2/5] fixed issue with missing keys in meta.yml --- nf_core/modules/lint.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py index 8212a77a99..fa51c52192 100644 --- a/nf_core/modules/lint.py +++ b/nf_core/modules/lint.py @@ -511,6 +511,7 @@ def lint_module_tests(self): def lint_meta_yml(self): """ Lint a meta yml file """ 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) @@ -526,7 +527,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: From 9b6d387674f08e5812e0f018206ff2bad4121d2a Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 19 Mar 2021 11:57:10 +0100 Subject: [PATCH 3/5] added LintResult object --- nf_core/modules/lint.py | 67 ++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py index fa51c52192..b07b28c1e5 100644 --- a/nf_core/modules/lint.py +++ b/nf_core/modules/lint.py @@ -34,6 +34,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' @@ -144,8 +155,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): """ @@ -174,9 +185,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] + passed = [LintResult(mod, m[0], m[1], m[2]) for m in passed] + warned = [LintResult(mod, m[0], m[1], m[2]) for m in warned] + failed = [LintResult(mod, m[0], m[1], m[2]) for m in failed] self.passed += passed self.warned += warned self.failed += failed @@ -266,8 +277,8 @@ def _print_results(self, show_passed=False): 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 @@ -281,17 +292,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}"), - Markdown(f"{result[1]}"), - os.path.relpath(result[2], self.dir), + 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 @@ -390,13 +401,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: @@ -406,24 +415,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)}", ) ) From 171315d8365a05a1b1460aec60c1b9730aca9f1c Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 19 Mar 2021 12:15:59 +0100 Subject: [PATCH 4/5] implemented sorting --- nf_core/modules/lint.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py index b07b28c1e5..c626353a6e 100644 --- a/nf_core/modules/lint.py +++ b/nf_core/modules/lint.py @@ -9,6 +9,7 @@ from __future__ import print_function import logging +import operator import os import questionary import re @@ -273,6 +274,11 @@ 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]): @@ -319,8 +325,8 @@ def _s(some_list): ) table = Table(style="green", box=rich.box.ROUNDED) table.add_column("Module name", width=max_mod_name_len) - table.add_column("Test message", no_wrap=True) - table.add_column("File path", no_wrap=True) + table.add_column("File path") + table.add_column("Test message") table = format_result(self.passed, table) console.print(table) @@ -333,8 +339,8 @@ def _s(some_list): ) table = Table(style="yellow", box=rich.box.ROUNDED) table.add_column("Module name", width=max_mod_name_len) - table.add_column("Test message", no_wrap=True) - table.add_column("File path", no_wrap=True) + table.add_column("File path") + table.add_column("Test message") table = format_result(self.warned, table) console.print(table) @@ -345,8 +351,8 @@ def _s(some_list): ) table = Table(style="red", box=rich.box.ROUNDED) table.add_column("Module name", width=max_mod_name_len) - table.add_column("Test message", no_wrap=True) - table.add_column("File path", no_wrap=True) + table.add_column("File path") + table.add_column("Test message") table = format_result(self.failed, table) console.print(table) From 0d5ba185a1fdce64896e1b923310004ff4b19f41 Mon Sep 17 00:00:00 2001 From: Kevin Menden Date: Fri, 19 Mar 2021 13:28:52 +0100 Subject: [PATCH 5/5] Update nf_core/modules/lint.py Co-authored-by: Phil Ewels --- nf_core/modules/lint.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py index 2c535d35d7..7772ea57a8 100644 --- a/nf_core/modules/lint.py +++ b/nf_core/modules/lint.py @@ -186,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 = [LintResult(mod, m[0], m[1], m[2]) for m in passed] - warned = [LintResult(mod, m[0], m[1], m[2]) for m in warned] - failed = [LintResult(mod, m[0], m[1], m[2]) 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): """