From 5ee9af351e514dc3c0ce552c49519dab3f037ed9 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 11:48:34 +0200 Subject: [PATCH 01/11] Typos and replace duplicated functions --- nf_core/modules/lint/__init__.py | 170 +++++++++++++++---------------- nf_core/modules/nfcore_module.py | 4 +- 2 files changed, 86 insertions(+), 88 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index f9762569f7..a6926ccdbc 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -176,85 +176,31 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F self.lint_tests.remove(test_name) # Lint local modules + print(local_modules) if local and len(local_modules) > 0: - self.lint_local_modules(local_modules) + self.lint_modules(local_modules, local=True) # Lint nf-core modules if len(nfcore_modules) > 0: - self.lint_nfcore_modules(nfcore_modules) + self.lint_modules(nfcore_modules, local=False) if print_results: self._print_results(show_passed=show_passed) return {"passed": self.passed, "warned": self.warned, "failed": self.failed} - def lint_local_modules(self, local_modules): - """ - Lint a local module - Only issues warnings instead of failures - """ - progress_bar = rich.progress.Progress( - "[bold blue]{task.description}", - rich.progress.BarColumn(bar_width=None), - "[magenta]{task.completed} of {task.total}[reset] » [bold yellow]{task.fields[test_name]}", - transient=True, - ) - with progress_bar: - lint_progress = progress_bar.add_task( - "Linting local modules", total=len(local_modules), test_name=os.path.basename(local_modules[0]) - ) - - for mod in local_modules: - progress_bar.update(lint_progress, advance=1, test_name=os.path.basename(mod)) - mod_object = NFCoreModule( - module_dir=mod, base_dir=self.dir, repo_type=self.repo_type, nf_core_module=False - ) - mod_object.main_nf = mod - mod_object.module_name = os.path.basename(mod) - self.lint_module(mod_object, local=True) - - def lint_nfcore_modules(self, nfcore_modules): - """ - Lint nf-core modules - For each nf-core module, checks for existence of the files - - main.nf - - meta.yml - - functions.nf - And verifies that their content. - - If the linting is run for modules in the central nf-core/modules repo - (repo_type==modules), files that are relevant for module testing are - also examined - """ - - progress_bar = rich.progress.Progress( - "[bold blue]{task.description}", - rich.progress.BarColumn(bar_width=None), - "[magenta]{task.completed} of {task.total}[reset] » [bold yellow]{task.fields[test_name]}", - transient=True, - ) - with progress_bar: - lint_progress = progress_bar.add_task( - "Linting nf-core modules", total=len(nfcore_modules), test_name=nfcore_modules[0].module_name - ) - for mod in nfcore_modules: - progress_bar.update(lint_progress, advance=1, test_name=mod.module_name) - self.lint_module(mod) - def get_installed_modules(self): """ - Make a list of all modules installed in this repository + Makes lists of the local and and nf-core modules installed in this directory. - Returns a tuple of two lists, one for local modules - and one for nf-core modules. The local modules are represented - as direct filepaths to the module '.nf' file. - Nf-core module are returned as file paths to the module directories. - In case the module contains several tools, one path to each tool directory - is returned. + Returns: + local_modules, nfcore_modules ([NfCoreModule], [NfCoreModule]): + A tuple of two lists: One for local modules and one for nf-core modules. + In case the module contains several subtools, one path to each tool directory + is returned. - returns (local_modules, nfcore_modules) """ - # initialize lists + # Initialize lists local_modules = [] nfcore_modules = [] local_modules_dir = None @@ -262,7 +208,7 @@ def get_installed_modules(self): # Get local modules if self.repo_type == "pipeline": - local_modules_dir = os.path.join(self.dir, "modules", "local", "process") + local_modules_dir = os.path.join(self.dir, "modules", "local") # Filter local modules if os.path.exists(local_modules_dir): @@ -288,15 +234,86 @@ def get_installed_modules(self): else: nfcore_modules.append(m) - # Make full (relative) file paths and create NFCoreModule objects - local_modules = [os.path.join(local_modules_dir, m) for m in local_modules] + # Create NFCoreModule objects for the nf-core and local modules nfcore_modules = [ NFCoreModule(os.path.join(nfcore_modules_dir, m), repo_type=self.repo_type, base_dir=self.dir) for m in nfcore_modules ] + local_modules = [ + NFCoreModule( + os.path.join(local_modules_dir, m), repo_type=self.repo_type, base_dir=self.dir, nf_core_module=False + ) + for m in local_modules + ] + + # The local modules mustn't conform to the same file structure + # as the nf-core modules. We therefore only check the main script + # of the module + for mod in local_modules: + mod.main_nf = mod.module_dir + mod.module_name = os.path.basename(mod.module_dir) + return local_modules, nfcore_modules + def lint_modules(self, modules, local=False): + """ + Lint a list of modules + + Args: + modules ([NFCoreModule]): A list of module objects + local (boolean): Whether the list consist of local or nf-core modules + """ + progress_bar = rich.progress.Progress( + "[bold blue]{task.description}", + rich.progress.BarColumn(bar_width=None), + "[magenta]{task.completed} of {task.total}[reset] » [bold yellow]{task.fields[test_name]}", + transient=True, + ) + with progress_bar: + lint_progress = progress_bar.add_task( + f"Linting {'local' if local else 'nf-core'} modules", + total=len(modules), + test_name=modules[0].module_name, + ) + + for mod in modules: + progress_bar.update(lint_progress, advance=1, test_name=mod.module_name) + self.lint_module(mod, local=local) + + def lint_module(self, mod, local=False): + """ + Perform linting on one module + + If the module is a local module we only check the `main.nf` file, + and it issue warnings instead of failures. + + If the module is a nf-core module we check for existence of the files + - main.nf + - meta.yml + - functions.nf + And verify that their content conform to the nf-core standards. + + If the linting is run for modules in the central nf-core/modules repo + (repo_type==modules), files that are relevant for module testing are + also examined + """ + + # Only check the main script in case of a local module + if local: + self.main_nf(mod) + self.passed += [LintResult(mod, *m) for m in mod.passed] + self.warned += [LintResult(mod, *m) for m in mod.warned] + + # Otherwise run all the lint tests + else: + for test_name in self.lint_tests: + getattr(self, test_name)(mod) + + self.passed += [LintResult(mod, *m) for m in mod.passed] + self.warned += [LintResult(mod, *m) for m in mod.warned] + self.failed += [LintResult(mod, *m) for m in mod.failed] + def _print_results(self, show_passed=False): """Print linting results to the command line. @@ -399,22 +416,3 @@ def _s(some_list): table.add_row(r"[!] {:>3} Test Warning{}".format(len(self.warned), _s(self.warned)), style="yellow") table.add_row(r"[✗] {:>3} Test{} Failed".format(len(self.failed), _s(self.failed)), style="red") console.print(table) - - def lint_module(self, mod, local=False): - """Perform linting on this module""" - # Iterate over modules and run all checks on them - - # Only check main_if in case of a local module - if local: - self.main_nf(mod) - self.passed += [LintResult(mod, m[0], m[1], m[2]) for m in mod.passed] - self.warned += [LintResult(mod, m[0], m[1], m[2]) for m in mod.warned] - - # Otherwise run all the lint tests - else: - for test_name in self.lint_tests: - getattr(self, test_name)(mod) - - self.passed += [LintResult(mod, m[0], m[1], m[2]) for m in mod.passed] - self.warned += [LintResult(mod, m[0], m[1], m[2]) for m in mod.warned] - self.failed += [LintResult(mod, m[0], m[1], m[2]) for m in mod.failed] diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index 28939579e0..9eb3feee56 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -1,12 +1,12 @@ """ -The NFCoreModule class holds information and uitily functions for a single module +The NFCoreModule class holds information and utility functions for a single module """ import os class NFCoreModule(object): """ - A class to hold the information a bout a nf-core module + A class to hold the information about a nf-core module Includes functionality for linting """ From 0050c75739aa3ad126fb33b1b4ee188ec360be7d Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 14:32:17 +0200 Subject: [PATCH 02/11] Break up code into functions --- nf_core/modules/lint/__init__.py | 91 ++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index a6926ccdbc..fb0fd25069 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -108,29 +108,33 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F :param print_results: Whether to print the linting results :param show_passed: Whether passed tests should be shown as well - :returns: dict of {passed, warned, failed} + :returns: A ModuleLint object containing information of + the passed, warned and failed tests """ - # Get list of all modules in a pipeline + # Get lists of all modules in a pipeline local_modules, nfcore_modules = self.get_installed_modules() # Prompt for module or all if module is None and not all_modules: - question = { - "type": "list", - "name": "all_modules", - "message": "Lint all modules or a single named module?", - "choices": ["All modules", "Named module"], - } - answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style) - if answer["all_modules"] == "All modules": - all_modules = True - else: - module = questionary.autocomplete( - "Tool name:", - choices=[m.module_name for m in nfcore_modules], - style=nf_core.utils.nfcore_question_style, - ).ask() + questions = [ + { + "type": "list", + "name": "all_modules", + "message": "Lint all modules or a single named module?", + "choices": ["All modules", "Named module"], + }, + { + "type": "autocomplete", + "name": "tool_name", + "message": "Tool name:", + "when": lambda x: x["all_modules"] == "Named module", + "choices": [m.module_name for m in nfcore_modules], + }, + ] + answers = questionary.unsafe_prompt(questions, style=nf_core.utils.nfcore_question_style) + all_modules = answers["all_modules"] == "All modules" + module = answers.get("tool_name") # Only lint the given module if module: @@ -148,35 +152,16 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F if module: log.info(f"Linting module: [magenta]{module}") - # Check that supplied test keys exist - bad_keys = [k for k in self.key if k not in self.lint_tests] - if len(bad_keys) > 0: - raise AssertionError( - "Test name{} not recognised: '{}'".format( - "s" if len(bad_keys) > 1 else "", - "', '".join(bad_keys), - ) - ) - - # If -k supplied, only run these tests + # Filter the tests by the key if one is supplied if self.key: + self.filter_tests_by_key() log.info("Only running tests: '{}'".format("', '".join(self.key))) - self.lint_tests = [k for k in self.lint_tests if k in self.key] # If it is a pipeline, load the lint config file and the modules.json file if self.repo_type == "pipeline": - self.load_lint_config() - self.modules_json = self.load_modules_json() - - # Only continue if a lint config has been loaded - if self.lint_config: - for test_name in self.lint_tests: - if self.lint_config.get(test_name, {}) is False: - log.info(f"Ignoring lint test: {test_name}") - self.lint_tests.remove(test_name) + self.set_up_pipeline_files() # Lint local modules - print(local_modules) if local and len(local_modules) > 0: self.lint_modules(local_modules, local=True) @@ -187,7 +172,33 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F if print_results: self._print_results(show_passed=show_passed) - return {"passed": self.passed, "warned": self.warned, "failed": self.failed} + return self + + def set_up_pipeline_files(self): + self.load_lint_config() + self.modules_json = self.load_modules_json() + + # Only continue if a lint config has been loaded + if self.lint_config: + for test_name in self.lint_tests: + if self.lint_config.get(test_name, {}) is False: + log.info(f"Ignoring lint test: {test_name}") + self.lint_tests.remove(test_name) + + def filter_tests_by_key(self): + """Filters the tests by the supplied key""" + # Check that supplied test keys exist + bad_keys = [k for k in self.key if k not in self.lint_tests] + if len(bad_keys) > 0: + raise AssertionError( + "Test name{} not recognised: '{}'".format( + "s" if len(bad_keys) > 1 else "", + "', '".join(bad_keys), + ) + ) + + # If -k supplied, only run these tests + self.lint_tests = [k for k in self.lint_tests if k in self.key] def get_installed_modules(self): """ From 4497775b571d6f98475adc85cba661c952bae5e6 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 14:51:43 +0200 Subject: [PATCH 03/11] Clean up code --- nf_core/modules/lint/__init__.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index fb0fd25069..fbc91c469c 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -78,6 +78,10 @@ def __init__(self, dir, key=()): self.failed = [] self.modules_repo = ModulesRepo() self.lint_tests = ["main_nf", "functions_nf", "meta_yml", "module_changes", "module_todos"] + + # Get lists of modules install in directory + self.all_local_modules, self.all_nfcore_modules = self.get_installed_modules() + self.key = key self.lint_config = None self.modules_json = None @@ -112,9 +116,6 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F the passed, warned and failed tests """ - # Get lists of all modules in a pipeline - local_modules, nfcore_modules = self.get_installed_modules() - # Prompt for module or all if module is None and not all_modules: questions = [ @@ -129,7 +130,7 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F "name": "tool_name", "message": "Tool name:", "when": lambda x: x["all_modules"] == "Named module", - "choices": [m.module_name for m in nfcore_modules], + "choices": [m.module_name for m in self.all_nfcore_modules], }, ] answers = questionary.unsafe_prompt(questions, style=nf_core.utils.nfcore_question_style) @@ -141,9 +142,12 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F if all_modules: raise ModuleLintException("You cannot specify a tool and request all tools to be linted.") local_modules = [] - nfcore_modules = [m for m in nfcore_modules if m.module_name == module] + nfcore_modules = [m for m in self.all_nfcore_modules if m.module_name == module] if len(nfcore_modules) == 0: raise ModuleLintException(f"Could not find the specified module: '{module}'") + else: + local_modules = self.all_local_modules + nfcore_modules = self.all_nfcore_modules if self.repo_type == "modules": log.info(f"Linting modules repo: [magenta]{self.dir}") @@ -172,8 +176,6 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F if print_results: self._print_results(show_passed=show_passed) - return self - def set_up_pipeline_files(self): self.load_lint_config() self.modules_json = self.load_modules_json() From a0f9a3a62f7c589c8da47855d9eb2bad0190afb8 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 16:08:21 +0200 Subject: [PATCH 04/11] Add header to lint results table and joint summary --- nf_core/__main__.py | 6 ++-- nf_core/lint/__init__.py | 61 +++++++++++++++++++++++--------- nf_core/lint_utils.py | 32 +++++++++++++++++ nf_core/modules/lint/__init__.py | 28 ++++++++++----- 4 files changed, 101 insertions(+), 26 deletions(-) create mode 100644 nf_core/lint_utils.py diff --git a/nf_core/__main__.py b/nf_core/__main__.py index fbce259f27..b702a4b6cb 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -333,8 +333,10 @@ def lint(dir, release, fix, key, show_passed, fail_ignored, markdown, json): # Run the lint tests! try: - lint_obj = nf_core.lint.run_linting(dir, release, fix, key, show_passed, fail_ignored, markdown, json) - if len(lint_obj.failed) > 0: + lint_obj, module_lint_obj = nf_core.lint.run_linting( + dir, release, fix, key, show_passed, fail_ignored, markdown, json + ) + if len(lint_obj.failed) + len(module_lint_obj.failed) > 0: sys.exit(1) except AssertionError as e: log.critical(e) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index b53e0ae6d7..69eac9e6e0 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -8,6 +8,7 @@ from rich.console import Console from rich.markdown import Markdown from rich.table import Table +from rich.panel import Panel import datetime import git import json @@ -19,6 +20,8 @@ import yaml import nf_core.utils +import nf_core.lint_utils +from nf_core.modules.lint import ModuleLint log = logging.getLogger(__name__) @@ -40,7 +43,7 @@ def run_linting( """ # Create the lint object - lint_obj = PipelineLint(pipeline_dir, release_mode, fix, key, fail_ignored) + lint_obj = PipelineLint(pipeline_dir, release_mode, fix, key, fail_ignored, show_passed) # Load the various pipeline configs lint_obj._load_lint_config() @@ -48,16 +51,31 @@ def run_linting( lint_obj._load_conda_environment() lint_obj._list_files() - # Run the linting tests + # Create the modules lint object + module_lint_obj = ModuleLint(pipeline_dir) + + # Run only the tests we want + module_lint_tests = ("module_changes",) + module_lint_obj.filter_tests_by_key(module_lint_tests) + + # Run the pipeline linting tests try: lint_obj._lint_pipeline() except AssertionError as e: log.critical("Critical error: {}".format(e)) log.info("Stopping tests...") - return lint_obj + return lint_obj, module_lint_obj + + # Run the module lint tests + if len(module_lint_obj.all_local_modules) > 0: + module_lint_obj.lint_modules(module_lint_obj.all_local_modules, local=True) + if len(module_lint_obj.all_nfcore_modules) > 0: + module_lint_obj.lint_modules(module_lint_obj.all_nfcore_modules, local=False) # Print the results lint_obj._print_results(show_passed) + module_lint_obj._print_results(show_passed) + nf_core.lint_utils.print_joint_summary(lint_obj, module_lint_obj) # Save results to Markdown file if md_fn is not None: @@ -75,7 +93,7 @@ def run_linting( if release_mode: log.info("Reminder: Lint tests were run in --release mode.") - return lint_obj + return lint_obj, module_lint_obj class PipelineLint(nf_core.utils.Pipeline): @@ -117,7 +135,7 @@ class PipelineLint(nf_core.utils.Pipeline): from .template_strings import template_strings from .version_consistency import version_consistency - def __init__(self, wf_path, release_mode=False, fix=(), key=(), fail_ignored=False): + def __init__(self, wf_path, release_mode=False, fix=(), key=(), fail_ignored=False, show_passed=False): """Initialise linting object""" # Initialise the parent object @@ -280,7 +298,7 @@ def _lint_pipeline(self): if test_results.get("could_fix", False): self.could_fix.append(test_name) - def _print_results(self, show_passed=False): + def _print_results(self, show_passed): """Print linting results to the command line. Uses the ``rich`` library to print a set of formatted tables to the command line @@ -305,6 +323,9 @@ def _s(some_list): return "s" return "" + # Print lint results header + console.print(Panel("[magenta]General lint results")) + # Table of passed tests if len(self.passed) > 0 and show_passed: table = Table(style="green", box=rich.box.ROUNDED) @@ -340,6 +361,24 @@ def _s(some_list): table = format_result(self.failed, table) console.print(table) + if len(self.could_fix): + fix_cmd = "nf-core lint {} --fix {}".format(self.wf_path, " --fix ".join(self.could_fix)) + console.print( + f"\nTip: Some of these linting errors can automatically be resolved with the following command:\n\n[blue] {fix_cmd}\n" + ) + if len(self.fix): + console.print( + "Automatic fixes applied. Please check with 'git diff' and revert any changes you do not want with 'git checkout '." + ) + + def _print_summary(self): + console = Console(force_terminal=nf_core.utils.rich_force_colors()) + + def _s(some_list): + if len(some_list) != 1: + return "s" + return "" + # Summary table summary_colour = "red" if len(self.failed) > 0 else "green" table = Table(box=rich.box.ROUNDED, style=summary_colour) @@ -352,16 +391,6 @@ def _s(some_list): table.add_row(r"[red][✗] {:>3} Test{} Failed".format(len(self.failed), _s(self.failed))) console.print(table) - if len(self.could_fix): - fix_cmd = "nf-core lint {} --fix {}".format(self.wf_path, " --fix ".join(self.could_fix)) - console.print( - f"\nTip: Some of these linting errors can automatically be resolved with the following command:\n\n[blue] {fix_cmd}\n" - ) - if len(self.fix): - console.print( - "Automatic fixes applied. Please check with 'git diff' and revert any changes you do not want with 'git checkout '." - ) - def _get_results_md(self): """ Create a markdown file suitable for posting in a GitHub comment. diff --git a/nf_core/lint_utils.py b/nf_core/lint_utils.py new file mode 100644 index 0000000000..b7d39f7587 --- /dev/null +++ b/nf_core/lint_utils.py @@ -0,0 +1,32 @@ +import rich +from rich.console import Console +from rich.table import Table +import logging + +import nf_core.utils + +log = logging.getLogger(__name__) + + +def print_joint_summary(lint_obj, module_lint_obj): + """Print a joint summary of the general pipe lint tests and the module lint tests""" + nbr_passed = len(lint_obj.passed) + len(module_lint_obj.passed) + nbr_ignored = len(lint_obj.ignored) + nbr_fixed = len(lint_obj.fixed) + nbr_warned = len(lint_obj.warned) + len(module_lint_obj.warned) + nbr_failed = len(lint_obj.failed) + len(module_lint_obj.failed) + + def _s(some_length): + return "" if some_length == 1 else "s" + + console = Console(force_terminal=nf_core.utils.rich_force_colors()) + summary_colour = "red" if nbr_failed > 0 else "green" + table = Table(box=rich.box.ROUNDED, style=summary_colour) + table.add_column(f"LINT RESULTS SUMMARY".format(nbr_passed), no_wrap=True) + table.add_row(r"[green][✔] {:>3} Test{} Passed".format(nbr_passed, _s(nbr_passed))) + if nbr_fixed: + table.add_row(r"[bright blue][?] {:>3} Test{} Fixed".format(nbr_fixed, _s(nbr_fixed))) + table.add_row(r"[grey58][?] {:>3} Test{} Ignored".format(nbr_ignored, _s(nbr_ignored))) + table.add_row(r"[yellow][!] {:>3} Test Warning{}".format(nbr_warned, _s(nbr_warned))) + table.add_row(r"[red][✗] {:>3} Test{} Failed".format(nbr_failed, _s(nbr_failed))) + console.print(table) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index fbc91c469c..6bcb33c970 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -21,6 +21,7 @@ from rich.console import Console from rich.table import Table from rich.markdown import Markdown +from rich.panel import Panel import rich from nf_core.utils import rich_force_colors from nf_core.lint.pipeline_todos import pipeline_todos @@ -66,7 +67,7 @@ class ModuleLint(ModuleCommand): from .module_todos import module_todos from .module_version import module_version - def __init__(self, dir, key=()): + def __init__(self, dir): self.dir = dir try: self.repo_type = nf_core.modules.module_utils.get_repo_type(self.dir) @@ -82,7 +83,6 @@ def __init__(self, dir, key=()): # Get lists of modules install in directory self.all_local_modules, self.all_nfcore_modules = self.get_installed_modules() - self.key = key self.lint_config = None self.modules_json = None @@ -94,7 +94,7 @@ def __init__(self, dir, key=()): # Add as first test to load git_sha before module_changes self.lint_tests.insert(0, "module_version") - def lint(self, module=None, all_modules=False, print_results=True, show_passed=False, local=False): + def lint(self, module=None, key=(), all_modules=False, print_results=True, show_passed=False, local=False): """ Lint all or one specific module @@ -157,9 +157,9 @@ def lint(self, module=None, all_modules=False, print_results=True, show_passed=F log.info(f"Linting module: [magenta]{module}") # Filter the tests by the key if one is supplied - if self.key: + if key: self.filter_tests_by_key() - log.info("Only running tests: '{}'".format("', '".join(self.key))) + log.info("Only running tests: '{}'".format("', '".join(key))) # If it is a pipeline, load the lint config file and the modules.json file if self.repo_type == "pipeline": @@ -187,10 +187,10 @@ def set_up_pipeline_files(self): log.info(f"Ignoring lint test: {test_name}") self.lint_tests.remove(test_name) - def filter_tests_by_key(self): + def filter_tests_by_key(self, key): """Filters the tests by the supplied key""" # Check that supplied test keys exist - bad_keys = [k for k in self.key if k not in self.lint_tests] + bad_keys = [k for k in key if k not in self.lint_tests] if len(bad_keys) > 0: raise AssertionError( "Test name{} not recognised: '{}'".format( @@ -200,7 +200,7 @@ def filter_tests_by_key(self): ) # If -k supplied, only run these tests - self.lint_tests = [k for k in self.lint_tests if k in self.key] + self.lint_tests = [k for k in self.lint_tests if k in key] def get_installed_modules(self): """ @@ -381,6 +381,9 @@ def _s(some_list): return "s" return "" + # Print module linting results header + console.print(Panel("[magenta]General lint results")) + # Table of passed tests if len(self.passed) > 0 and show_passed: console.print( @@ -419,6 +422,15 @@ def _s(some_list): table = format_result(self.failed, table) console.print(table) + def print_summary(self): + + console = Console(force_terminal=rich_force_colors()) + + def _s(some_list): + if len(some_list) > 1: + return "s" + return "" + # Summary table table = Table(box=rich.box.ROUNDED) table.add_column("[bold green]LINT RESULTS SUMMARY".format(len(self.passed)), no_wrap=True) From 70d1b1eeffa4cf9da95267dbb6d8480c79facadb Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 16:31:25 +0200 Subject: [PATCH 05/11] Moved message about fix option last in lint output --- nf_core/lint/__init__.py | 21 ++++++--------------- nf_core/lint_utils.py | 18 +++++++++++++++++- nf_core/modules/lint/__init__.py | 7 ++----- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 69eac9e6e0..c2517c5e48 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -5,7 +5,6 @@ the nf-core community guidelines. """ -from rich.console import Console from rich.markdown import Markdown from rich.table import Table from rich.panel import Panel @@ -21,6 +20,7 @@ import nf_core.utils import nf_core.lint_utils +from nf_core.lint_utils import console from nf_core.modules.lint import ModuleLint log = logging.getLogger(__name__) @@ -55,9 +55,12 @@ def run_linting( module_lint_obj = ModuleLint(pipeline_dir) # Run only the tests we want - module_lint_tests = ("module_changes",) + module_lint_tests = ("module_changes", "module_version") module_lint_obj.filter_tests_by_key(module_lint_tests) + # Set up files for modules linting test + module_lint_obj.set_up_pipeline_files() + # Run the pipeline linting tests try: lint_obj._lint_pipeline() @@ -76,6 +79,7 @@ def run_linting( lint_obj._print_results(show_passed) module_lint_obj._print_results(show_passed) nf_core.lint_utils.print_joint_summary(lint_obj, module_lint_obj) + nf_core.lint_utils.print_fixes(lint_obj, module_lint_obj) # Save results to Markdown file if md_fn is not None: @@ -306,7 +310,6 @@ def _print_results(self, show_passed): """ log.debug("Printing final results") - console = Console(force_terminal=nf_core.utils.rich_force_colors()) # Helper function to format test links nicely def format_result(test_results, table): @@ -361,19 +364,7 @@ def _s(some_list): table = format_result(self.failed, table) console.print(table) - if len(self.could_fix): - fix_cmd = "nf-core lint {} --fix {}".format(self.wf_path, " --fix ".join(self.could_fix)) - console.print( - f"\nTip: Some of these linting errors can automatically be resolved with the following command:\n\n[blue] {fix_cmd}\n" - ) - if len(self.fix): - console.print( - "Automatic fixes applied. Please check with 'git diff' and revert any changes you do not want with 'git checkout '." - ) - def _print_summary(self): - console = Console(force_terminal=nf_core.utils.rich_force_colors()) - def _s(some_list): if len(some_list) != 1: return "s" diff --git a/nf_core/lint_utils.py b/nf_core/lint_utils.py index b7d39f7587..6e2084922e 100644 --- a/nf_core/lint_utils.py +++ b/nf_core/lint_utils.py @@ -7,6 +7,9 @@ log = logging.getLogger(__name__) +# Create a console used by all lint tests +console = Console(force_terminal=nf_core.utils.rich_force_colors()) + def print_joint_summary(lint_obj, module_lint_obj): """Print a joint summary of the general pipe lint tests and the module lint tests""" @@ -19,7 +22,6 @@ def print_joint_summary(lint_obj, module_lint_obj): def _s(some_length): return "" if some_length == 1 else "s" - console = Console(force_terminal=nf_core.utils.rich_force_colors()) summary_colour = "red" if nbr_failed > 0 else "green" table = Table(box=rich.box.ROUNDED, style=summary_colour) table.add_column(f"LINT RESULTS SUMMARY".format(nbr_passed), no_wrap=True) @@ -30,3 +32,17 @@ def _s(some_length): table.add_row(r"[yellow][!] {:>3} Test Warning{}".format(nbr_warned, _s(nbr_warned))) table.add_row(r"[red][✗] {:>3} Test{} Failed".format(nbr_failed, _s(nbr_failed))) console.print(table) + + +def print_fixes(lint_obj, module_lint_obj): + """Prints available and applied fixes""" + + if len(lint_obj.could_fix): + fix_cmd = "nf-core lint {} --fix {}".format(lint_obj.wf_path, " --fix ".join(lint_obj.could_fix)) + console.print( + f"\nTip: Some of these linting errors can automatically be resolved with the following command:\n\n[blue] {fix_cmd}\n" + ) + if len(lint_obj.fix): + console.print( + "Automatic fixes applied. Please check with 'git diff' and revert any changes you do not want with 'git checkout '." + ) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 6bcb33c970..c84f181948 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -18,7 +18,6 @@ import rich import yaml import json -from rich.console import Console from rich.table import Table from rich.markdown import Markdown from rich.panel import Panel @@ -29,8 +28,10 @@ import nf_core.utils import nf_core.modules.module_utils + from nf_core.modules.modules_repo import ModulesRepo from nf_core.modules.nfcore_module import NFCoreModule +from nf_core.lint_utils import console log = logging.getLogger(__name__) @@ -335,7 +336,6 @@ 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")) @@ -423,9 +423,6 @@ def _s(some_list): console.print(table) def print_summary(self): - - console = Console(force_terminal=rich_force_colors()) - def _s(some_list): if len(some_list) > 1: return "s" From 842a6c329f8c267c1bbec1ac651e6df364194cac Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 6 Jul 2021 16:50:33 +0200 Subject: [PATCH 06/11] small fixes --- nf_core/lint/__init__.py | 4 ++-- nf_core/modules/lint/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index c2517c5e48..d0b61be7c7 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -43,7 +43,7 @@ def run_linting( """ # Create the lint object - lint_obj = PipelineLint(pipeline_dir, release_mode, fix, key, fail_ignored, show_passed) + lint_obj = PipelineLint(pipeline_dir, release_mode, fix, key, fail_ignored) # Load the various pipeline configs lint_obj._load_lint_config() @@ -139,7 +139,7 @@ class PipelineLint(nf_core.utils.Pipeline): from .template_strings import template_strings from .version_consistency import version_consistency - def __init__(self, wf_path, release_mode=False, fix=(), key=(), fail_ignored=False, show_passed=False): + def __init__(self, wf_path, release_mode=False, fix=(), key=(), fail_ignored=False): """Initialise linting object""" # Initialise the parent object diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index c84f181948..d3e6cd5daf 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -382,7 +382,7 @@ def _s(some_list): return "" # Print module linting results header - console.print(Panel("[magenta]General lint results")) + console.print(Panel("[magenta]Module lint results")) # Table of passed tests if len(self.passed) > 0 and show_passed: From 348856b27c17b2eecb41034b09265d0b0dcd053b Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Fri, 9 Jul 2021 11:00:41 +0200 Subject: [PATCH 07/11] Apply changes from code review --- nf_core/modules/lint/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 15a39ab890..4b097d9207 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -159,7 +159,7 @@ def lint(self, module=None, key=(), all_modules=False, print_results=True, show_ # Filter the tests by the key if one is supplied if key: - self.filter_tests_by_key() + self.filter_tests_by_key(key) log.info("Only running tests: '{}'".format("', '".join(key))) # If it is a pipeline, load the lint config file and the modules.json file From f3bf644a4df1e2aa2105ce0d5e66f9f7fd9d3869 Mon Sep 17 00:00:00 2001 From: Erik Danielsson <53212377+ErikDanielsson@users.noreply.github.com> Date: Fri, 9 Jul 2021 11:01:12 +0200 Subject: [PATCH 08/11] Update nf_core/modules/lint/__init__.py Co-authored-by: Kevin Menden --- nf_core/modules/lint/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 4b097d9207..246c6c80da 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -300,7 +300,7 @@ def lint_module(self, mod, local=False): Perform linting on one module If the module is a local module we only check the `main.nf` file, - and it issue warnings instead of failures. + and issue warnings instead of failures. If the module is a nf-core module we check for existence of the files - main.nf From ffcb0556dface07cdf3da99c7ef8b5f45a4fad5b Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Fri, 9 Jul 2021 11:10:37 +0200 Subject: [PATCH 09/11] Fix errors with 'key' --- nf_core/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 2855b04afe..425ce9a45b 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -543,9 +543,9 @@ def lint(ctx, tool, dir, key, all, local, passed): nf-core/modules repository. """ try: - module_lint = nf_core.modules.ModuleLint(dir=dir, key=key) + module_lint = nf_core.modules.ModuleLint(dir=dir) module_lint.modules_repo = ctx.obj["modules_repo_obj"] - module_lint.lint(module=tool, all_modules=all, print_results=True, local=local, show_passed=passed) + module_lint.lint(module=tool, key=key, all_modules=all, print_results=True, local=local, show_passed=passed) if len(module_lint.failed) > 0: sys.exit(1) except nf_core.modules.lint.ModuleLintException as e: From 87d85a1258ef04860a2cb76a181afd0ffa1f1b5e Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Fri, 9 Jul 2021 11:19:16 +0200 Subject: [PATCH 10/11] Print lint summary in modules lint --- nf_core/modules/lint/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 246c6c80da..af5ed69722 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -422,6 +422,9 @@ def _s(some_list): table = format_result(self.failed, table) console.print(table) + # Print lint summary + self.print_summary() + def print_summary(self): def _s(some_list): if len(some_list) > 1: From 0060db2e3d41aeabb1338d450b61cfc8226bce63 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Fri, 9 Jul 2021 11:23:46 +0200 Subject: [PATCH 11/11] Fix summary output --- nf_core/modules/lint/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index af5ed69722..c870d136d3 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -176,6 +176,7 @@ def lint(self, module=None, key=(), all_modules=False, print_results=True, show_ if print_results: self._print_results(show_passed=show_passed) + self.print_summary() def set_up_pipeline_files(self): self.load_lint_config() @@ -422,9 +423,6 @@ def _s(some_list): table = format_result(self.failed, table) console.print(table) - # Print lint summary - self.print_summary() - def print_summary(self): def _s(some_list): if len(some_list) > 1: