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] diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 105dde08dd..229bc79e59 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -114,8 +114,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/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 8cce9d5bba..e0e4374994 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -12,6 +12,7 @@ import logging import operator import os +from pathlib import Path import questionary import rich @@ -77,7 +78,30 @@ 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_nfcore_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: + 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: + module_dir = Path(self.dir, "modules") + self.all_remote_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 @@ -143,7 +167,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) @@ -155,12 +179,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] - 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_nfcore_modules + remote_modules = self.all_remote_modules if self.repo_type == "modules": log.info(f"Linting modules repo: [magenta]'{self.dir}'") @@ -183,8 +207,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) @@ -217,78 +241,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 and nf-core 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. - - """ - # 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 - 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 - ] - - 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, fix_version=False): """ Lint a list of modules 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/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/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/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/module_utils.py b/nf_core/modules/module_utils.py index 97346ed169..144f7ce3d4 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,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(os.path.join(nfcore_modules_dir, m), repo_type=repo_type, base_dir=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/modules_command.py b/nf_core/modules/modules_command.py index fca80dd7e8..2f59dbd41c 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 str(Path(dir).relative_to(module_base_path)).startswith("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") diff --git a/nf_core/modules/nfcore_module.py b/nf_core/modules/nfcore_module.py index 5ec871929c..84925f6200 100644 --- a/nf_core/modules/nfcore_module.py +++ b/nf_core/modules/nfcore_module.py @@ -11,7 +11,21 @@ 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 + + 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_name = module_name + self.repo_name = repo_name self.module_dir = module_dir self.repo_type = repo_type self.base_dir = base_dir @@ -27,24 +41,26 @@ def __init__(self, module_dir, repo_type, base_dir, nf_core_module=True): if nf_core_module: # Initialize the important files - self.main_nf = str(Path(self.module_dir, "main.nf")) - self.meta_yml = str(Path(self.module_dir, "meta.yml")) - if self.repo_type == "pipeline": - self.module_name = module_dir.split("nf-core/modules" + os.sep)[1] - else: - if "modules/modules" in module_dir: - self.module_name = module_dir.split("modules/modules" + os.sep)[1] - else: - self.module_name = module_dir.split("modules" + os.sep)[1] + self.main_nf = self.module_dir / "main.nf" + self.meta_yml = self.module_dir / "meta.yml" - 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" - # Check if we have a patch file 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: + # 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.meta_yml = None + self.test_dir = None + self.test_yml = None + self.test_main_nf = None 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 (