From 88bd6d32044cd1f1ba9cb0cf47e1447fb4261388 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Fri, 29 Jul 2022 16:03:51 +0200 Subject: [PATCH 1/7] Start adding support for custom remotes --- nf_core/lint/__init__.py | 4 +- nf_core/modules/lint/__init__.py | 79 ++++++++++-------------------- nf_core/modules/list.py | 1 + nf_core/modules/modules_command.py | 45 ++++++++++------- 4 files changed, 55 insertions(+), 74 deletions(-) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 27619ad73f..325e999528 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -111,8 +111,8 @@ def run_linting( # 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) + if len(module_lint_obj.all_remote_modules) > 0: + module_lint_obj.lint_modules(module_lint_obj.all_remote_modules, local=False) # Print the results lint_obj._print_results(show_passed) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index a4664ee05a..663d5cc2fb 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -15,6 +15,7 @@ import os import re import sys +from pathlib import Path import questionary import requests @@ -84,7 +85,7 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull self.modules_repo = ModulesRepo(remote_url, branch, no_pull, base_path) self.lint_tests = self._get_all_lint_tests() # Get lists of modules install in directory - self.all_local_modules, self.all_nfcore_modules = self.get_installed_modules() + self.all_local_modules, self.all_remote_modules = self.get_installed_modules() self.lint_config = None self.modules_json = None @@ -149,7 +150,7 @@ def lint( "name": "tool_name", "message": "Tool name:", "when": lambda x: x["all_modules"] == "Named module", - "choices": [m.module_name for m in self.all_nfcore_modules], + "choices": [m.module_name for m in self.all_remote_modules], }, ] answers = questionary.unsafe_prompt(questions, style=nf_core.utils.nfcore_question_style) @@ -161,12 +162,12 @@ def lint( 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 self.all_nfcore_modules if m.module_name == module] + nfcore_modules = [m for m in self.all_remote_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 + nfcore_modules = self.all_remote_modules if self.repo_type == "modules": log.info(f"Linting modules repo: [magenta]'{self.dir}'") @@ -225,7 +226,7 @@ def filter_tests_by_key(self, key): def get_installed_modules(self): """ - Makes lists of the local and and nf-core modules installed in this directory. + Makes lists of the local and remote modules installed in this directory. Returns: local_modules, nfcore_modules ([NfCoreModule], [NfCoreModule]): @@ -234,56 +235,28 @@ def get_installed_modules(self): is returned. """ - # Initialize lists - local_modules = [] - nfcore_modules = [] - local_modules_dir = None - nfcore_modules_dir = os.path.join(self.dir, "modules", "nf-core", "modules") - - # Get local modules + self.get_pipeline_modules() if self.repo_type == "pipeline": - local_modules_dir = os.path.join(self.dir, "modules", "local") - - # Filter local modules - if os.path.exists(local_modules_dir): - local_modules = sorted([x for x in os.listdir(local_modules_dir) if x.endswith(".nf")]) - - # nf-core/modules - if self.repo_type == "modules": - nfcore_modules_dir = os.path.join(self.dir, "modules") - - # Get nf-core modules - if os.path.exists(nfcore_modules_dir): - for m in sorted(os.listdir(nfcore_modules_dir)): - if not os.path.isdir(os.path.join(nfcore_modules_dir, m)): - raise ModuleLintException( - f"File found in '{nfcore_modules_dir}': '{m}'! " - "This directory should only contain module directories." - ) - - module_dir = os.path.join(nfcore_modules_dir, m) - module_subdir = os.listdir(module_dir) - # Not a module, but contains sub-modules - if "main.nf" not in module_subdir: - for path in module_subdir: - module_subdir_path = os.path.join(nfcore_modules_dir, m, path) - if os.path.isdir(module_subdir_path): - if os.path.exists(os.path.join(module_subdir_path, "main.nf")): - nfcore_modules.append(os.path.join(m, path)) - else: - nfcore_modules.append(m) - - # 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 - ] + remote_modules = { + repo_name: [ + NFCoreModule(Path(self.dir, "modules", repo_name, m), repo_type=self.repo_type, base_dir=self.dir) + for m in modules + ] + for repo_name, modules in self.module_names.items() + if repo_name != "local" + } + else: + remote_modules = { + "modules": [ + NFCoreModule(Path(self.dir, "modules", m), repo_type=self.repo_type, base_dir=self.dir) + for m in modules + ] + for modules in self.module_names["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 + NFCoreModule(Path(self.dir, "local", m), repo_type=self.repo_type, base_dir=self.dir, nf_core_module=False) + for m in self.module_names.get("local", []) ] # The local modules mustn't conform to the same file structure @@ -293,7 +266,7 @@ def get_installed_modules(self): mod.main_nf = mod.module_dir mod.module_name = os.path.basename(mod.module_dir) - return local_modules, nfcore_modules + return local_modules, remote_modules def lint_modules(self, modules, local=False, fix_version=False): """ diff --git a/nf_core/modules/list.py b/nf_core/modules/list.py index 355daedd4f..f52de10427 100644 --- a/nf_core/modules/list.py +++ b/nf_core/modules/list.py @@ -75,6 +75,7 @@ def pattern_msg(keywords): repos_with_mods = { repo_name: [mod for mod in self.module_names[repo_name] if all(k in mod for k in keywords)] for repo_name in self.module_names + if repo_name != "local" } # Nothing found diff --git a/nf_core/modules/modules_command.py b/nf_core/modules/modules_command.py index fca80dd7e8..dda84c2001 100644 --- a/nf_core/modules/modules_command.py +++ b/nf_core/modules/modules_command.py @@ -2,6 +2,7 @@ import logging import os import shutil +from pathlib import Path import yaml @@ -25,7 +26,7 @@ def __init__(self, dir, remote_url=None, branch=None, no_pull=False, base_path=N """ self.modules_repo = ModulesRepo(remote_url, branch, no_pull, base_path) self.dir = dir - self.module_names = [] + self.module_names = None try: if self.dir: self.dir, self.repo_type = nf_core.modules.module_utils.get_repo_type(self.dir) @@ -34,7 +35,7 @@ def __init__(self, dir, remote_url=None, branch=None, no_pull=False, base_path=N except LookupError as e: raise UserWarning(e) - def get_pipeline_modules(self): + def get_pipeline_modules(self, local=False): """ Get the modules installed in the current directory. @@ -48,29 +49,35 @@ def get_pipeline_modules(self): self.module_names = {} - module_base_path = f"{self.dir}/modules/" + module_base_path = Path(self.dir, "modules") if self.repo_type == "pipeline": - repo_owners = (owner for owner in os.listdir(module_base_path) if owner != "local") - repo_names = ( - f"{repo_owner}/{name}" - for repo_owner in repo_owners - for name in os.listdir(f"{module_base_path}/{repo_owner}") + module_relpaths = ( + Path(dir).relative_to(module_base_path) + for dir, _, files in os.walk(module_base_path) + if "main.nf" in files and not Path(dir).relative_to(module_base_path).is_relative_to("local") ) - for repo_name in repo_names: - repo_path = os.path.join(module_base_path, repo_name) - module_mains_path = f"{repo_path}/**/main.nf" - module_mains = glob.glob(module_mains_path, recursive=True) - if len(module_mains) > 0: - self.module_names[repo_name] = [ - os.path.dirname(os.path.relpath(mod, repo_path)) for mod in module_mains - ] + # The two highest directories are the repo owner and repo name. The rest is the module name + repos_and_modules = (("/".join(dir.parts[:2]), "/".join(dir.parts[2:])) for dir in module_relpaths) + for repo, module in repos_and_modules: + if repo not in self.module_names: + self.module_names[repo] = [] + self.module_names[repo].append(module) + + local_module_dir = Path(module_base_path, "local") + if local and local_module_dir.exists(): + # Get the local modules + self.module_names["local"] = [ + str(path.relative_to(local_module_dir)) + for path in local_module_dir.iterdir() + if path.suffix == ".nf" + ] elif self.repo_type == "modules": - module_mains_path = f"{module_base_path}/**/main.nf" - module_mains = glob.glob(module_mains_path, recursive=True) self.module_names["modules"] = [ - os.path.dirname(os.path.relpath(mod, module_base_path)) for mod in module_mains + str(Path(dir).relative_to(module_base_path)) + for dir, _, files in os.walk(module_base_path) + if "main.nf" in files ] else: log.error("Directory is neither a clone of nf-core/modules nor a pipeline") From a912e9307dd3c6a84a713b13040350acd5dc89ef Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Mon, 1 Aug 2022 15:48:48 +0200 Subject: [PATCH 2/7] Use pathlib.Path in NFCoreModule --- nf_core/modules/bump_versions.py | 3 +-- nf_core/modules/lint/__init__.py | 10 +++++--- nf_core/modules/module_utils.py | 3 ++- nf_core/modules/nfcore_module.py | 40 +++++++++++++++++++++++++------- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/nf_core/modules/bump_versions.py b/nf_core/modules/bump_versions.py index 0d7826009e..bf5d946586 100644 --- a/nf_core/modules/bump_versions.py +++ b/nf_core/modules/bump_versions.py @@ -17,7 +17,6 @@ import nf_core.modules.module_utils import nf_core.utils -from nf_core.modules.nfcore_module import NFCoreModule from nf_core.utils import plural_s as _s from nf_core.utils import rich_force_colors @@ -112,7 +111,7 @@ def bump_versions(self, module=None, all_modules=False, show_uptodate=False): self._print_results() - def bump_module_version(self, module: NFCoreModule): + def bump_module_version(self, module): """ Bump the bioconda and container version of a single NFCoreModule diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 663d5cc2fb..55c6dbea08 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -239,7 +239,9 @@ def get_installed_modules(self): if self.repo_type == "pipeline": remote_modules = { repo_name: [ - NFCoreModule(Path(self.dir, "modules", repo_name, m), repo_type=self.repo_type, base_dir=self.dir) + NFCoreModule( + Path(self.dir, "modules", repo_name, m), repo_type=self.repo_type, base_dir=Path(self.dir) + ) for m in modules ] for repo_name, modules in self.module_names.items() @@ -248,14 +250,16 @@ def get_installed_modules(self): else: remote_modules = { "modules": [ - NFCoreModule(Path(self.dir, "modules", m), repo_type=self.repo_type, base_dir=self.dir) + NFCoreModule(Path(self.dir, "modules", m), repo_type=self.repo_type, base_dir=Path(self.dir)) for m in modules ] for modules in self.module_names["modules"] } local_modules = [ - NFCoreModule(Path(self.dir, "local", m), repo_type=self.repo_type, base_dir=self.dir, nf_core_module=False) + NFCoreModule( + Path(self.dir, "local", m), repo_type=self.repo_type, base_dir=Path(self.dir), nf_core_module=False + ) for m in self.module_names.get("local", []) ] diff --git a/nf_core/modules/module_utils.py b/nf_core/modules/module_utils.py index 97346ed169..2a751e9bfa 100644 --- a/nf_core/modules/module_utils.py +++ b/nf_core/modules/module_utils.py @@ -1,6 +1,7 @@ import logging import os import urllib +from pathlib import Path import questionary import rich @@ -90,7 +91,7 @@ def get_installed_modules(dir, repo_type="modules"): # Make full (relative) file paths and create NFCoreModule objects local_modules = [os.path.join(local_modules_dir, m) for m in local_modules] nfcore_modules = [ - NFCoreModule(os.path.join(nfcore_modules_dir, m), repo_type=repo_type, base_dir=dir) for m in nfcore_modules + NFCoreModule(Path(nfcore_modules_dir, m), repo_type=repo_type, base_dir=Path(dir)) for m in nfcore_modules ] return local_modules, nfcore_modules diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index f828142fda..b0a9feceaa 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -2,6 +2,7 @@ The NFCoreModule class holds information and utility functions for a single module """ import os +from pathlib import Path class NFCoreModule(object): @@ -11,6 +12,18 @@ class NFCoreModule(object): """ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): + """ + Initialize the object + + Args: + module_dir (Path): The absolute path to the module + repo_type (str): Either 'pipeline' or 'modules' depending on + whether the directory is a pipeline or clone + of nf-core/modules. + base_dir (Path): The absolute path to the pipeline base dir + nf_core_module (bool): Whether the module is to be treated as a + nf-core or local module + """ self.module_dir = module_dir self.repo_type = repo_type self.base_dir = base_dir @@ -24,16 +37,25 @@ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): if nf_core_module: # Initialize the important files - self.main_nf = os.path.join(self.module_dir, "main.nf") - self.meta_yml = os.path.join(self.module_dir, "meta.yml") + self.main_nf = self.module_dir / "main.nf" + self.meta_yml = self.module_dir / "meta.yml" if self.repo_type == "pipeline": - self.module_name = module_dir.split("nf-core/modules" + os.sep)[1] + self.module_name = str(self.module_dir.relative_to(self.base_dir / "nf-core/modules")) else: - if "modules/modules" in module_dir: - self.module_name = module_dir.split("modules/modules" + os.sep)[1] + if "modules/modules" in str(module_dir): + self.module_name = str(self.module_dir.relative_to(self.base_dir / "modules/modules")) else: - self.module_name = module_dir.split("modules" + os.sep)[1] + self.module_name = str(self.module_dir.relative_to(self.base_dir / "modules")) - self.test_dir = os.path.join(self.base_dir, "tests", "modules", self.module_name) - self.test_yml = os.path.join(self.test_dir, "test.yml") - self.test_main_nf = os.path.join(self.test_dir, "main.nf") + self.test_dir = Path(self.base_dir, "tests", "modules", self.module_name) + self.test_yml = self.test_dir / "test.yml" + self.test_main_nf = self.test_dir / "main.nf" + else: + # These attributes are only used by nf-core modules + # so just initialize them to None + self.module_name = None + self.main_nf = None + self.meta_yml = None + self.test_dir = None + self.test_yml = None + self.test_main_nf = None From 993b3e343c2f785ead6bf30a375c40d7689537cf Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Mon, 1 Aug 2022 16:11:12 +0200 Subject: [PATCH 3/7] Use ModulesCommand method for retrieving pipeline modules --- nf_core/modules/lint/__init__.py | 60 ++++++-------------------------- nf_core/modules/nfcore_module.py | 7 ++++ 2 files changed, 18 insertions(+), 49 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index b075253f0a..5ac063fb44 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -78,7 +78,17 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull self.modules_repo = ModulesRepo(remote_url, branch, no_pull, base_path) self.lint_tests = self.get_all_lint_tests(self.repo_type == "pipeline") # Get lists of modules install in directory - self.all_local_modules, self.all_remote_modules = self.get_installed_modules() + self.get_pipeline_modules(local=True) + + if self.repo_type == "pipeline": + if self.modules_repo.fullname in self.module_names: + self.all_remote_modules = self.module_names[self.modules_repo.fullname] + else: + raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.") + else: + self.all_remote_modules = self.module_names["modules"] + + self.local_modules = self.module_names.get("local", []) self.lint_config = None self.modules_json = None @@ -218,54 +228,6 @@ def filter_tests_by_key(self, key): # If -k supplied, only run these tests self.lint_tests = [k for k in self.lint_tests if k in key] - def get_installed_modules(self): - """ - Makes lists of the local and remote modules installed in this directory. - - 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. - - """ - self.get_pipeline_modules() - if self.repo_type == "pipeline": - remote_modules = { - repo_name: [ - NFCoreModule( - Path(self.dir, "modules", repo_name, m), repo_type=self.repo_type, base_dir=Path(self.dir) - ) - for m in modules - ] - for repo_name, modules in self.module_names.items() - if repo_name != "local" - } - else: - remote_modules = { - "modules": [ - NFCoreModule(Path(self.dir, "modules", m), repo_type=self.repo_type, base_dir=Path(self.dir)) - for m in modules - ] - for modules in self.module_names["modules"] - } - - local_modules = [ - NFCoreModule( - Path(self.dir, "local", m), repo_type=self.repo_type, base_dir=Path(self.dir), nf_core_module=False - ) - for m in self.module_names.get("local", []) - ] - - # 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, remote_modules - def lint_modules(self, modules, local=False, fix_version=False): """ Lint a list of modules diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index 2a23baee20..ac5d2174de 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -52,6 +52,13 @@ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): self.test_dir = Path(self.base_dir, "tests", "modules", self.module_name) self.test_yml = self.test_dir / "test.yml" self.test_main_nf = self.test_dir / "main.nf" + + if self.repo_type == "pipeline": + patch_fn = f"{self.module_name.replace('/', '-')}.diff" + patch_path = Path(self.module_dir, patch_fn) + if patch_path.exists(): + self.is_patched = True + self.patch_path = patch_path else: # These attributes are only used by nf-core modules # so just initialize them to None From 0d6e4edfa066d5a400281818a4f47314f460e19d Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 2 Aug 2022 12:16:38 +0200 Subject: [PATCH 4/7] Use repo name instead of nf-core/modules --- nf_core/modules/lint/__init__.py | 28 +++++++++++++++++--------- nf_core/modules/lint/main_nf.py | 2 +- nf_core/modules/lint/module_changes.py | 2 +- nf_core/modules/lint/module_patch.py | 6 +++--- nf_core/modules/lint/module_version.py | 3 ++- nf_core/modules/module_utils.py | 3 ++- nf_core/modules/nfcore_module.py | 11 +++------- 7 files changed, 31 insertions(+), 24 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 5ac063fb44..43d19419f3 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -82,13 +82,23 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull if self.repo_type == "pipeline": if self.modules_repo.fullname in self.module_names: - self.all_remote_modules = self.module_names[self.modules_repo.fullname] + module_dir = Path(self.dir, "modules", self.modules_repo.fullname) + self.all_remote_modules = [ + NFCoreModule(m, self.modules_repo.fullname, module_dir / m, self.repo_type, Path(self.dir)) + for m in self.module_names[self.modules_repo.fullname] + ] + local_module_dir = Path(self.dir, "modules", "local") + self.all_local_modules = [ + NFCoreModule(m, None, local_module_dir / m, self.repo_type, Path(self.dir), nf_core_module=False) + for m in self.module_names.get("local", []) + ] + else: raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.") else: - self.all_remote_modules = self.module_names["modules"] - - self.local_modules = self.module_names.get("local", []) + self.all_remote_modules = [ + NFCoreModule(module_dir / m, self.repo_type, Path(self.dir)) for m in self.module_names["modules"] + ] self.lint_config = None self.modules_json = None @@ -166,12 +176,12 @@ def lint( 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 self.all_remote_modules if m.module_name == module] - if len(nfcore_modules) == 0: + remote_modules = [m for m in self.all_remote_modules if m.module_name == module] + if len(remote_modules) == 0: raise ModuleLintException(f"Could not find the specified module: '{module}'") else: local_modules = self.all_local_modules - nfcore_modules = self.all_remote_modules + remote_modules = self.all_remote_modules if self.repo_type == "modules": log.info(f"Linting modules repo: [magenta]'{self.dir}'") @@ -194,8 +204,8 @@ def lint( self.lint_modules(local_modules, local=True, fix_version=fix_version) # Lint nf-core modules - if len(nfcore_modules) > 0: - self.lint_modules(nfcore_modules, local=False, fix_version=fix_version) + if len(remote_modules) > 0: + self.lint_modules(remote_modules, local=False, fix_version=fix_version) if print_results: self._print_results(show_passed=show_passed) diff --git a/nf_core/modules/lint/main_nf.py b/nf_core/modules/lint/main_nf.py index 12fe2c2eb0..0b302431eb 100644 --- a/nf_core/modules/lint/main_nf.py +++ b/nf_core/modules/lint/main_nf.py @@ -46,7 +46,7 @@ def main_nf(module_lint_object, module, fix_version, progress_bar): if module.is_patched: lines = ModulesDiffer.try_apply_patch( module.module_name, - "nf-core/modules", + module_lint_object.modules_repo.fullname, module.patch_path, Path(module.module_dir).relative_to(module.base_dir), reverse=True, diff --git a/nf_core/modules/lint/module_changes.py b/nf_core/modules/lint/module_changes.py index 8addb9a7bf..972f1b4504 100644 --- a/nf_core/modules/lint/module_changes.py +++ b/nf_core/modules/lint/module_changes.py @@ -29,7 +29,7 @@ def module_changes(module_lint_object, module): shutil.copytree(module.module_dir, tempdir, dirs_exist_ok=True) try: new_lines = ModulesDiffer.try_apply_patch( - module.module_name, "nf-core/modules", module.patch_path, tempdir, reverse=True + module.module_name, module_lint_object.modules_repo.fullname, module.patch_path, tempdir, reverse=True ) for file, lines in new_lines.items(): with open(tempdir / file, "w") as fh: diff --git a/nf_core/modules/lint/module_patch.py b/nf_core/modules/lint/module_patch.py index cbdd85869b..a74b2e2344 100644 --- a/nf_core/modules/lint/module_patch.py +++ b/nf_core/modules/lint/module_patch.py @@ -22,7 +22,7 @@ def module_patch(module_lint_obj, module: NFCoreModule): # Test failed, just exit return - patch_reversible(module, module.patch_path) + patch_reversible(module_lint_obj, module, module.patch_path) def check_patch_valid(module, patch_path): @@ -151,7 +151,7 @@ def check_patch_valid(module, patch_path): return passed -def patch_reversible(module, patch_path): +def patch_reversible(module_lint_object, module, patch_path): """ Try applying a patch in reverse to see if it is up to date @@ -165,7 +165,7 @@ def patch_reversible(module, patch_path): try: ModulesDiffer.try_apply_patch( module.module_name, - "nf-core/modules", + module_lint_object.modules_repo.fullname, patch_path, Path(module.module_dir).relative_to(module.base_dir), reverse=True, diff --git a/nf_core/modules/lint/module_version.py b/nf_core/modules/lint/module_version.py index a5886f6022..af18786c53 100644 --- a/nf_core/modules/lint/module_version.py +++ b/nf_core/modules/lint/module_version.py @@ -5,6 +5,7 @@ import logging import os +from pathlib import Path import nf_core import nf_core.modules.module_utils @@ -22,7 +23,7 @@ def module_version(module_lint_object, module): newer version of the module available. """ - modules_json_path = os.path.join(module_lint_object.dir, "modules.json") + modules_json_path = Path(module_lint_object.dir, "modules.json") # Verify that a git_sha exists in the `modules.json` file for this module module_version = module_lint_object.modules_json.get_module_version( diff --git a/nf_core/modules/module_utils.py b/nf_core/modules/module_utils.py index 2a751e9bfa..144f7ce3d4 100644 --- a/nf_core/modules/module_utils.py +++ b/nf_core/modules/module_utils.py @@ -91,7 +91,8 @@ def get_installed_modules(dir, repo_type="modules"): # Make full (relative) file paths and create NFCoreModule objects local_modules = [os.path.join(local_modules_dir, m) for m in local_modules] nfcore_modules = [ - NFCoreModule(Path(nfcore_modules_dir, m), repo_type=repo_type, base_dir=Path(dir)) for m in nfcore_modules + NFCoreModule(m, "nf-core/modules", Path(nfcore_modules_dir, m), repo_type=repo_type, base_dir=Path(dir)) + for m in nfcore_modules ] return local_modules, nfcore_modules diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index ac5d2174de..42c78fbaa6 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -11,7 +11,7 @@ class NFCoreModule(object): Includes functionality for linting """ - def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): + def __init__(self, module_name, repo_name, module_dir, repo_type, base_dir, nf_core_module=True): """ Initialize the object @@ -24,6 +24,8 @@ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): nf_core_module (bool): Whether the module is to be treated as a nf-core or local module """ + self.module_name = module_name + self.repo_name = repo_name self.module_dir = module_dir self.repo_type = repo_type self.base_dir = base_dir @@ -41,13 +43,6 @@ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): # Initialize the important files self.main_nf = self.module_dir / "main.nf" self.meta_yml = self.module_dir / "meta.yml" - if self.repo_type == "pipeline": - self.module_name = str(self.module_dir.relative_to(self.base_dir / "nf-core/modules")) - else: - if "modules/modules" in str(module_dir): - self.module_name = str(self.module_dir.relative_to(self.base_dir / "modules/modules")) - else: - self.module_name = str(self.module_dir.relative_to(self.base_dir / "modules")) self.test_dir = Path(self.base_dir, "tests", "modules", self.module_name) self.test_yml = self.test_dir / "test.yml" From cd8a599ff24fb712af376111cea1c58347445266 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 2 Aug 2022 13:53:45 +0200 Subject: [PATCH 5/7] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b94aff1cb5..6e94af1db4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ - Add `nf-core modules patch` command ([#1312](https://github.com/nf-core/tools/issues/1312)) - Add support for patch in `nf-core modules update` command ([#1312](https://github.com/nf-core/tools/issues/1312)) - Add support for patch in `nf-core modules lint` command ([#1312](https://github.com/nf-core/tools/issues/1312)) +- Add support for custom remotes in `nf-core modules lint` ([#1715](https://github.com/nf-core/tools/issues/1715)) ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16] From f2bd50bb84f706505dd0010b5d77010051126be1 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 2 Aug 2022 15:33:15 +0200 Subject: [PATCH 6/7] Don't use is_relative_to for backwards compatibility --- nf_core/modules/modules_command.py | 2 +- nf_core/modules/nfcore_module.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/modules_command.py b/nf_core/modules/modules_command.py index dda84c2001..2f59dbd41c 100644 --- a/nf_core/modules/modules_command.py +++ b/nf_core/modules/modules_command.py @@ -55,7 +55,7 @@ def get_pipeline_modules(self, local=False): module_relpaths = ( Path(dir).relative_to(module_base_path) for dir, _, files in os.walk(module_base_path) - if "main.nf" in files and not Path(dir).relative_to(module_base_path).is_relative_to("local") + if "main.nf" in files and not str(Path(dir).relative_to(module_base_path)).startswith("local") ) # The two highest directories are the repo owner and repo name. The rest is the module name repos_and_modules = (("/".join(dir.parts[:2]), "/".join(dir.parts[2:])) for dir in module_relpaths) diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index 42c78fbaa6..84925f6200 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -55,10 +55,11 @@ def __init__(self, module_name, repo_name, module_dir, repo_type, base_dir, nf_c self.is_patched = True self.patch_path = patch_path else: + # The main file is just the local module + self.main_nf = self.module_dir + self.module_name = self.module_dir.stem # These attributes are only used by nf-core modules # so just initialize them to None - self.module_name = None - self.main_nf = None self.meta_yml = None self.test_dir = None self.test_yml = None From dd2032d584e6b909812a36828531b4ce2f084035 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Tue, 2 Aug 2022 16:41:14 +0200 Subject: [PATCH 7/7] Fix failing tests --- nf_core/modules/lint/__init__.py | 5 ++++- nf_core/modules/lint/meta_yml.py | 2 +- tests/modules/lint.py | 28 +++++++++++++++++++++++----- tests/test_modules.py | 2 ++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index 43d19419f3..e0e4374994 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -96,9 +96,12 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull else: raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.") else: + module_dir = Path(self.dir, "modules") self.all_remote_modules = [ - NFCoreModule(module_dir / m, self.repo_type, Path(self.dir)) for m in self.module_names["modules"] + NFCoreModule(m, None, module_dir / m, self.repo_type, Path(self.dir)) + for m in self.module_names["modules"] ] + self.all_local_modules = [] self.lint_config = None self.modules_json = None diff --git a/nf_core/modules/lint/meta_yml.py b/nf_core/modules/lint/meta_yml.py index 42f5ac21a1..146796fde2 100644 --- a/nf_core/modules/lint/meta_yml.py +++ b/nf_core/modules/lint/meta_yml.py @@ -28,7 +28,7 @@ def meta_yml(module_lint_object, module): if module.is_patched: lines = ModulesDiffer.try_apply_patch( module.module_name, - "nf-core/modules", + module_lint_object.modules_repo.fullname, module.patch_path, Path(module.module_dir).relative_to(module.base_dir), reverse=True, diff --git a/tests/modules/lint.py b/tests/modules/lint.py index 0f60377d5e..64b5f2ff0f 100644 --- a/tests/modules/lint.py +++ b/tests/modules/lint.py @@ -1,5 +1,9 @@ +import pytest + import nf_core.modules +from ..utils import GITLAB_URL + def test_modules_lint_trimgalore(self): """Test linting the TrimGalore! module""" @@ -16,11 +20,8 @@ def test_modules_lint_empty(self): self.mods_remove.remove("fastqc") self.mods_remove.remove("multiqc") self.mods_remove.remove("custom/dumpsoftwareversions") - module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir) - module_lint.lint(print_results=False, all_modules=True) - assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}" - assert len(module_lint.passed) == 0 - assert len(module_lint.warned) == 0 + with pytest.raises(LookupError): + nf_core.modules.ModuleLint(dir=self.pipeline_dir) def test_modules_lint_new_modules(self): @@ -30,3 +31,20 @@ def test_modules_lint_new_modules(self): assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}" assert len(module_lint.passed) > 0 assert len(module_lint.warned) >= 0 + + +def test_modules_lint_no_gitlab(self): + """Test linting a pipeline with no modules installed""" + with pytest.raises(LookupError): + nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) + + +def test_modules_lint_gitlab_modules(self): + """Lint modules from a different remote""" + self.mods_install_gitlab.install("fastqc") + self.mods_install_gitlab.install("multiqc") + module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) + module_lint.lint(print_results=False, all_modules=True) + assert len(module_lint.failed) == 0 + assert len(module_lint.passed) > 0 + assert len(module_lint.warned) >= 0 diff --git a/tests/test_modules.py b/tests/test_modules.py index beb6c54abf..c3a9abfbf8 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -110,7 +110,9 @@ def test_modulesrepo_class(self): ) from .modules.lint import ( test_modules_lint_empty, + test_modules_lint_gitlab_modules, test_modules_lint_new_modules, + test_modules_lint_no_gitlab, test_modules_lint_trimgalore, ) from .modules.list import (