diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e94af1db4..e6ea0eecdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ - 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)) +- Make `nf-core modules` commands work with arbitrary git remotes ([#1721](https://github.com/nf-core/tools/issues/1721)) ## [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/modules_json.py b/nf_core/lint/modules_json.py index 43c1f93c9f..4cb5e2cfa7 100644 --- a/nf_core/lint/modules_json.py +++ b/nf_core/lint/modules_json.py @@ -1,5 +1,7 @@ #!/usr/bin/env python +from pathlib import Path + from nf_core.modules.modules_command import ModuleCommand from nf_core.modules.modules_json import ModulesJson @@ -21,10 +23,9 @@ def modules_json(self): modules_json = ModulesJson(self.wf_path) modules_json.load() modules_json_dict = modules_json.modules_json + modules_dir = Path(self.wf_path, "modules") if modules_json: - modules_command.get_pipeline_modules() - all_modules_passed = True for repo in modules_json_dict["repos"].keys(): @@ -35,9 +36,11 @@ def modules_json(self): ) continue - for key in modules_json_dict["repos"][repo]["modules"]: - if not key in modules_command.module_names[repo]: - failed.append(f"Entry for `{key}` found in `modules.json` but module is not installed in pipeline.") + for module in modules_json_dict["repos"][repo]["modules"]: + if not Path(modules_dir, repo, module).exists(): + failed.append( + f"Entry for `{Path(repo, module)}` found in `modules.json` but module is not installed in pipeline." + ) all_modules_passed = False if all_modules_passed: diff --git a/nf_core/modules/info.py b/nf_core/modules/info.py index 7dac5067bb..febdee3589 100644 --- a/nf_core/modules/info.py +++ b/nf_core/modules/info.py @@ -11,6 +11,7 @@ from rich.text import Text import nf_core.utils +from nf_core.modules.modules_json import ModulesJson from .module_utils import get_repo_type from .modules_command import ModuleCommand @@ -35,7 +36,11 @@ def __init__(self, pipeline_dir, tool, remote_url, branch, no_pull, base_path): log.debug(f"Only showing remote info: {e}") pipeline_dir = None - self.get_pipeline_modules() + if self.repo_type == "pipeline": + self.modules_json = ModulesJson(self.dir) + self.modules_json.check_up_to_date() + else: + self.modules_json = None self.module = self.init_mod_name(tool) def init_mod_name(self, module): @@ -51,9 +56,9 @@ def init_mod_name(self, module): ).unsafe_ask() if local: if self.repo_type == "modules": - modules = self.module_names["modules"] + modules = self.get_modules_clone_modules() else: - modules = self.module_names.get(self.modules_repo.fullname) + modules = self.modules_json.get_all_modules().get(self.modules_repo.fullname) if modules is None: raise UserWarning(f"No modules installed from '{self.modules_repo.remote_url}'") else: @@ -98,7 +103,7 @@ def get_local_yaml(self): repo_name = self.modules_repo.fullname module_base_path = os.path.join(self.dir, "modules", repo_name) # Check that we have any modules installed from this repo - modules = self.module_names.get(repo_name) + modules = self.modules_json.get_all_modules().get(repo_name) if modules is None: raise LookupError(f"No modules installed from {self.modules_repo.remote_url}") diff --git a/nf_core/modules/install.py b/nf_core/modules/install.py index e6322f3ff2..6f013ce3b9 100644 --- a/nf_core/modules/install.py +++ b/nf_core/modules/install.py @@ -118,8 +118,7 @@ def install(self, module): return False else: # Fetch the latest commit for the module - git_log = list(self.modules_repo.get_module_git_log(module, depth=1)) - version = git_log[0]["git_sha"] + version = self.modules_repo.get_latest_module_version(module) if self.force: log.info(f"Removing installed version of '{self.modules_repo.fullname}/{module}'") diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index e0e4374994..8fb6fecc2c 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -77,20 +77,23 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull self.failed = [] 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.get_pipeline_modules(local=True) if self.repo_type == "pipeline": - if self.modules_repo.fullname in self.module_names: + modules_json = ModulesJson(self.dir) + modules_json.check_up_to_date() + all_pipeline_modules = modules_json.get_all_modules() + if self.modules_repo.fullname in all_pipeline_modules: 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] + for m in all_pipeline_modules[self.modules_repo.fullname] ] + if not self.all_remote_modules: + raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.") 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", []) + for m in self.get_local_modules() ] else: @@ -99,9 +102,11 @@ def __init__(self, dir, fail_warned=False, remote_url=None, branch=None, no_pull 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"] + for m in self.get_modules_clone_modules() ] self.all_local_modules = [] + if not self.all_remote_modules: + raise LookupError("No modules in 'modules' directory") self.lint_config = None self.modules_json = None diff --git a/nf_core/modules/list.py b/nf_core/modules/list.py index f52de10427..7ae3e08962 100644 --- a/nf_core/modules/list.py +++ b/nf_core/modules/list.py @@ -68,14 +68,10 @@ def pattern_msg(keywords): modules_json = ModulesJson(self.dir) modules_json.check_up_to_date() - # Get installed modules - self.get_pipeline_modules() - # Filter by 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" + repo_name: [mod for mod in modules if all(k in mod for k in keywords)] + for repo_name, modules in modules_json.get_all_modules().items() } # Nothing found diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 2311416fe1..927fd6b693 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -16,6 +16,7 @@ import nf_core.modules.module_utils import nf_core.utils from nf_core.modules.modules_command import ModuleCommand +from nf_core.modules.modules_json import ModulesJson log = logging.getLogger(__name__) @@ -61,7 +62,6 @@ def __init__( self.pytest_args = pytest_args super().__init__(".", remote_url, branch, no_pull) - self.get_pipeline_modules() def run(self): """Run test steps""" @@ -79,9 +79,11 @@ def _check_inputs(self): # Retrieving installed modules if self.repo_type == "modules": - installed_modules = self.module_names["modules"] + installed_modules = self.get_modules_clone_modules() else: - installed_modules = self.module_names.get(self.modules_repo.fullname) + modules_json = ModulesJson(self.dir) + modules_json.check_up_to_date() + installed_modules = modules_json.get_all_modules().get(self.modules_repo.fullname) # Get the tool name if not specified if self.module_name is None: @@ -89,7 +91,7 @@ def _check_inputs(self): raise UserWarning( "Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL." ) - if installed_modules is None: + if not installed_modules: raise UserWarning( f"No installed modules were found from '{self.modules_repo.remote_url}'.\n" f"Are you running the tests inside the nf-core/modules main directory?\n" diff --git a/nf_core/modules/modules_command.py b/nf_core/modules/modules_command.py index 2f59dbd41c..7c2d313449 100644 --- a/nf_core/modules/modules_command.py +++ b/nf_core/modules/modules_command.py @@ -26,7 +26,6 @@ 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 = None try: if self.dir: self.dir, self.repo_type = nf_core.modules.module_utils.get_repo_type(self.dir) @@ -35,53 +34,23 @@ 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, local=False): + def get_modules_clone_modules(self): """ - Get the modules installed in the current directory. - - If the current directory is a pipeline, the `module_names` - field is set to a dictionary indexed by the different - installation repositories in the directory. If the directory - is a clone of nf-core/modules the filed is set to - `{"modules": modules_in_dir}` - + Get the modules available in a clone of nf-core/modules """ - - self.module_names = {} - module_base_path = Path(self.dir, "modules") + return [ + str(Path(dir).relative_to(module_base_path)) + for dir, _, files in os.walk(module_base_path) + if "main.nf" in files + ] - if self.repo_type == "pipeline": - 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") - ) - # 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": - self.module_names["modules"] = [ - 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") - raise SystemError + def get_local_modules(self): + """ + Get the local modules in a pipeline + """ + local_module_dir = Path(self.dir, "modules", "local") + return [str(path.relative_to(local_module_dir)) for path in local_module_dir.iterdir() if path.suffix == ".nf"] def has_valid_directory(self): """Check that we were given a pipeline or clone of nf-core/modules""" @@ -123,6 +92,24 @@ def clear_module_dir(self, module_name, module_dir): log.error(f"Could not remove module: {e}") return False + def modules_from_repo(self, repo_name): + """ + Gets the modules installed from a certain repository + + Args: + repo_name (str): The name of the repository + + Returns: + [str]: The names of the modules + """ + repo_dir = Path(self.dir, "modules", repo_name) + if not repo_dir.exists(): + raise LookupError(f"Nothing installed from {repo_name} in pipeline") + + return [ + str(Path(dir_path).relative_to(repo_dir)) for dir_path, _, files in os.walk(repo_dir) if "main.nf" in files + ] + def install_module_files(self, module_name, module_version, modules_repo, install_dir): """ Installs a module into the given directory diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index b86b2250fa..92cadd0a1e 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -32,6 +32,7 @@ def __init__(self, pipeline_dir): self.dir = pipeline_dir self.modules_dir = os.path.join(self.dir, "modules") self.modules_json = None + self.pipeline_modules = None def create(self): """ @@ -49,7 +50,7 @@ def create(self): if not os.path.exists(modules_dir): raise UserWarning("Can't find a ./modules directory. Is this a DSL2 pipeline?") - repos = self.get_pipeline_module_repositories(modules_dir) + repos = self.get_pipeline_module_repositories(Path(modules_dir)) # Get all module names in the repos repo_module_names = [ @@ -106,7 +107,7 @@ def get_pipeline_module_repositories(self, modules_dir, repos=None): """ Finds all module repositories in the modules directory. Ignores the local modules. Args: - modules_dir (str): base directory for the module files + modules_dir (Path): base directory for the module files Returns repos [ (str, str, str) ]: List of tuples of repo name, repo remote URL and path to modules in repo """ @@ -114,21 +115,22 @@ def get_pipeline_module_repositories(self, modules_dir, repos=None): repos = {} # Check if there are any nf-core modules installed - if os.path.exists(os.path.join(modules_dir, nf_core.modules.modules_repo.NF_CORE_MODULES_NAME)): + if (modules_dir / nf_core.modules.modules_repo.NF_CORE_MODULES_NAME).exists(): repos[nf_core.modules.modules_repo.NF_CORE_MODULES_NAME] = ( nf_core.modules.modules_repo.NF_CORE_MODULES_REMOTE, nf_core.modules.modules_repo.NF_CORE_MODULES_BASE_PATH, ) # Check if there are any untracked repositories - dirs_not_covered = self.dir_tree_uncovered(modules_dir, [name for name in repos]) + dirs_not_covered = self.dir_tree_uncovered(modules_dir, [Path(name) for name in repos]) if len(dirs_not_covered) > 0: log.info("Found custom module repositories when creating 'modules.json'") # Loop until all directories in the base directory are covered by a remote while len(dirs_not_covered) > 0: log.info( "The following director{s} in the modules directory are untracked: '{l}'".format( - s="ies" if len(dirs_not_covered) > 0 else "y", l="', '".join(dirs_not_covered) + s="ies" if len(dirs_not_covered) > 0 else "y", + l="', '".join(str(dir.relative_to(modules_dir)) for dir in dirs_not_covered), ) ) nrepo_remote = questionary.text( @@ -146,7 +148,7 @@ def get_pipeline_module_repositories(self, modules_dir, repos=None): # Verify that there is a directory corresponding the remote nrepo_name = nf_core.modules.module_utils.path_from_remote(nrepo_remote) - if not os.path.exists(os.path.join(modules_dir, nrepo_name)): + if not (modules_dir / nrepo_name).exists(): log.info( "The provided remote does not seem to correspond to a local directory. " "The directory structure should be the same as in the remote." @@ -155,7 +157,7 @@ def get_pipeline_module_repositories(self, modules_dir, repos=None): "Please provide the correct directory, it will be renamed. If left empty, the remote will be ignored." ).unsafe_ask() if dir_name: - os.rename(os.path.join(modules_dir, dir_name), os.path.join(modules_dir, nrepo_name)) + (modules_dir, dir_name).rename(modules_dir / nrepo_name) else: continue @@ -168,7 +170,7 @@ def get_pipeline_module_repositories(self, modules_dir, repos=None): nrepo_base_path = nf_core.modules.modules_repo.NF_CORE_MODULES_BASE_PATH repos[nrepo_name] = (nrepo_remote, nrepo_base_path) - dirs_not_covered = self.dir_tree_uncovered(modules_dir, [name for name in repos]) + dirs_not_covered = self.dir_tree_uncovered(modules_dir, [Path(name) for name in repos]) return repos def find_correct_commit_sha(self, module_name, module_path, modules_repo): @@ -199,26 +201,27 @@ def dir_tree_uncovered(self, modules_dir, repos): subdirectories are therefore ignore. Args: - module_dir (str): Base path of modules in pipeline - repos ([ str ]): List of repos that are covered by a remote + module_dir (Path): Base path of modules in pipeline + repos ([ Path ]): List of repos that are covered by a remote Returns: - dirs_not_covered ([ str ]): A list of directories that are currently not covered by any remote. + dirs_not_covered ([ Path ]): A list of directories that are currently not covered by any remote. """ + # Initialise the FIFO queue. Note that we assume the directory to be correctly # configured, i.e. no files etc. - fifo = [os.path.join(modules_dir, subdir) for subdir in os.listdir(modules_dir) if subdir != "local"] + fifo = [subdir for subdir in modules_dir.iterdir() if subdir.stem != "local"] depth = 1 dirs_not_covered = [] while len(fifo) > 0: temp_queue = [] - repos_at_level = {os.path.join(*os.path.split(repo)[:depth]): len(os.path.split(repo)) for repo in repos} + repos_at_level = {Path(*repo.parts[:depth]): len(repo.parts) for repo in repos} for dir in fifo: - rel_dir = os.path.relpath(dir, modules_dir) + rel_dir = dir.relative_to(modules_dir) if rel_dir in repos_at_level.keys(): # Go the next depth if this directory is not one of the repos if depth < repos_at_level[rel_dir]: - temp_queue.extend([os.path.join(dir, subdir) for subdir in os.listdir(dir)]) + temp_queue.extend(dir.iterdir()) else: # Otherwise add the directory to the ones not covered dirs_not_covered.append(dir) @@ -649,6 +652,24 @@ def get_base_path(self, repo_name): self.load() return self.modules_json.get("repos", {}).get(repo_name, {}).get("base_path", None) + def get_all_modules(self): + """ + Retrieves all pipeline modules that are reported in the modules.json + + Returns: + (dict[str, [str]]): Dictionary indexed with the repo names, with a + list of modules as values + """ + if self.modules_json is None: + self.load() + if self.pipeline_modules is None: + self.pipeline_modules = {} + for repo, repo_entry in self.modules_json.get("repos", {}).items(): + if "modules" in repo_entry: + self.pipeline_modules[repo] = list(repo_entry["modules"]) + + return self.pipeline_modules + def dump(self): """ Sort the modules.json, and write it to file diff --git a/nf_core/modules/patch.py b/nf_core/modules/patch.py index c276d4e146..8104a939ea 100644 --- a/nf_core/modules/patch.py +++ b/nf_core/modules/patch.py @@ -20,21 +20,23 @@ def __init__(self, dir, remote_url=None, branch=None, no_pull=False, base_path=N super().__init__(dir, remote_url, branch, no_pull, base_path) self.modules_json = ModulesJson(dir) - self.get_pipeline_modules() def param_check(self, module): if not self.has_valid_directory(): raise UserWarning() - if module is not None and module not in self.module_names[self.modules_repo.fullname]: + if module is not None and module not in self.modules_json.get_all_modules().get(self.modules_repo.fullname, {}): raise UserWarning(f"Module '{Path(self.modules_repo.fullname, module)}' does not exist in the pipeline") def patch(self, module=None): + self.modules_json.check_up_to_date() self.param_check(module) if module is None: module = questionary.autocomplete( - "Tool:", self.module_names[self.modules_repo.fullname], style=nf_core.utils.nfcore_question_style + "Tool:", + self.modules_json.get_all_modules()[self.modules_repo.fullname], + style=nf_core.utils.nfcore_question_style, ).unsafe_ask() module_fullname = str(Path(self.modules_repo.fullname, module)) diff --git a/nf_core/modules/remove.py b/nf_core/modules/remove.py index 5857ebf2b9..4b14745a92 100644 --- a/nf_core/modules/remove.py +++ b/nf_core/modules/remove.py @@ -1,5 +1,6 @@ import logging import os +from pathlib import Path import questionary @@ -30,40 +31,23 @@ def remove(self, module): # Check whether pipelines is valid self.has_valid_directory() - # Get the installed modules - self.get_pipeline_modules() - if sum(map(len, self.module_names)) == 0: - log.error("No installed modules found in pipeline") - return False - - # Decide from which repo the module was installed - # TODO Configure the prompt for repository name in a nice way - if True: - repo_name = self.modules_repo.fullname - elif len(self.module_names) == 1: - repo_name = list(self.module_names.keys())[0] - else: - repo_name = questionary.autocomplete( - "Repo name:", choices=self.module_names.keys(), style=nf_core.utils.nfcore_question_style - ).unsafe_ask() - + repo_name = self.modules_repo.fullname if module is None: module = questionary.autocomplete( - "Tool name:", choices=self.module_names[repo_name], style=nf_core.utils.nfcore_question_style + "Tool name:", + choices=self.modules_from_repo(repo_name), + style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - # Set the remove folder based on the repository name - remove_folder = os.path.split(repo_name) - # Get the module directory - module_dir = os.path.join(self.dir, "modules", *remove_folder, module) + module_dir = Path(self.dir, "modules", repo_name, module) # Load the modules.json file modules_json = ModulesJson(self.dir) modules_json.load() # Verify that the module is actually installed - if not os.path.exists(module_dir): + if not module_dir.exists(): log.error(f"Module directory does not exist: '{module_dir}'") if modules_json.module_present(module, repo_name): diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index a6204c85f2..48e322abe0 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -46,9 +46,6 @@ def __init__( self.update_config = None self.modules_json = ModulesJson(self.dir) - # Fetch the list of pipeline modules - self.get_pipeline_modules() - class DiffEnum(enum.Enum): """Enumeration to keeping track of file diffs. @@ -279,18 +276,18 @@ def get_single_module_info(self, module): """ # Check if there are any modules installed from the repo repo_name = self.modules_repo.fullname - if repo_name not in self.module_names: + if repo_name not in self.modules_json.get_all_modules(): raise LookupError(f"No modules installed from '{repo_name}'") if module is None: module = questionary.autocomplete( "Tool name:", - choices=self.module_names[repo_name], + choices=self.modules_json.get_all_modules()[repo_name], style=nf_core.utils.nfcore_question_style, ).unsafe_ask() # Check if module is installed before trying to update - if module not in self.module_names[repo_name]: + if module not in self.modules_json.get_all_modules()[repo_name]: raise LookupError(f"Module '{module}' is not installed in pipeline and could therefore not be updated") # Check that the supplied name is an available module @@ -340,7 +337,7 @@ def get_all_modules_info(self): modules_info = {} # Loop through all the modules in the pipeline # and check if they have an entry in the '.nf-core.yml' file - for repo_name, modules in self.module_names.items(): + for repo_name, modules in self.modules_json.get_all_modules().items(): if repo_name not in self.update_config or self.update_config[repo_name] is True: modules_info[repo_name] = [(module, self.sha) for module in modules] elif isinstance(self.update_config[repo_name], dict): diff --git a/tests/modules/module_test.py b/tests/modules/module_test.py index c29297392a..41babfb604 100644 --- a/tests/modules/module_test.py +++ b/tests/modules/module_test.py @@ -1,5 +1,7 @@ """Test the 'modules test' command which runs module pytests.""" import os +import shutil +from pathlib import Path import pytest @@ -32,8 +34,10 @@ def test_modules_test_no_installed_modules(self): """Test the check_inputs() function - raise UserWarning because installed modules were not found""" cwd = os.getcwd() os.chdir(self.nfcore_modules) + module_dir = Path(self.nfcore_modules, "modules") + shutil.rmtree(module_dir) + module_dir.mkdir() meta_builder = nf_core.modules.ModulesTest(None, False, "") - meta_builder.module_names["modules"] = None meta_builder.repo_type = "modules" with pytest.raises(UserWarning) as excinfo: meta_builder._check_inputs()