diff --git a/CHANGELOG.md b/CHANGELOG.md index f56a185ba7..af9a486b38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ - Simplify control flow and don't use equality comparison for `None` and booleans - Replace use of the deprecated `distutils` Version object with that from `packaging` ([#1735](https://github.com/nf-core/tools/pull/1735)) - Add code to cancel CI run if a new run starts ([#1760](https://github.com/nf-core/tools/pull/1760)) +- CI for the API docs generation now uses the ubuntu-latest base image ([#1762](https://github.com/nf-core/tools/pull/1762)) ### Modules @@ -75,6 +76,7 @@ - Add `branch` field to module entries in `modules.json` to record what branch a module was installed from ([#1728](https://github.com/nf-core/tools/issues/1728)) - Fix broken link in `nf-core modules info`([#1745](https://github.com/nf-core/tools/pull/1745)) - Fix unbound variable issues and minor refactoring [#1742](https://github.com/nf-core/tools/pull/1742/) +- Recreate modules.json file instead of complaining about incorrectly formatted file. ([#1741](https://github.com/nf-core/tools/pull/1741) - Add support for patch when creating `modules.json` file ([#1752](https://github.com/nf-core/tools/pull/1752)) ## [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 539389c538..73d858608b 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -87,8 +87,9 @@ def run_linting( # Create the modules lint object module_lint_obj = nf_core.modules.lint.ModuleLint(pipeline_dir) - # Verify that the pipeline is correctly configured + # Verify that the pipeline is correctly configured and has a modules.json file module_lint_obj.has_valid_directory() + module_lint_obj.has_modules_file() # Run only the tests we want if key: diff --git a/nf_core/modules/list.py b/nf_core/modules/list.py index 1c6a151126..f063f21151 100644 --- a/nf_core/modules/list.py +++ b/nf_core/modules/list.py @@ -91,10 +91,6 @@ def pattern_msg(keywords): repo_entry = modules_json["repos"].get(repo_name, {}) for module in sorted(modules): repo_modules = repo_entry.get("modules") - if repo_modules is None: - raise UserWarning( - "You 'modules.json' file is not up to date. Please remove it and rerun the command" - ) module_entry = repo_modules.get(module) if module_entry: diff --git a/nf_core/modules/modules_command.py b/nf_core/modules/modules_command.py index 9d2798e4be..dbfd1ff6dd 100644 --- a/nf_core/modules/modules_command.py +++ b/nf_core/modules/modules_command.py @@ -62,7 +62,6 @@ def has_valid_directory(self): nf_config = os.path.join(self.dir, "nextflow.config") if not os.path.exists(main_nf) and not os.path.exists(nf_config): raise UserWarning(f"Could not find a 'main.nf' or 'nextflow.config' file in '{self.dir}'") - self.has_modules_file() return True def has_modules_file(self): diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index edf1ca2a3a..fbc9f4a8f2 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -76,11 +76,12 @@ def create(self): modules_json["repos"][repo_name]["modules"] = self.determine_module_branches_and_shas( repo_name, remote_url, module_names ) - + # write the modules.json file and assign it to the object modules_json_path = Path(self.dir, "modules.json") with open(modules_json_path, "w") as fh: json.dump(modules_json, fh, indent=4) fh.write("\n") + self.modules_json = modules_json def get_pipeline_module_repositories(self, modules_dir, repos=None): """ @@ -199,7 +200,6 @@ def determine_module_branches_and_shas(self, repo_name, remote_url, modules): Args: repo_name (str): The name of the module repository remote_url (str): The url to the remote repository - modules_base_path (Path): The path to the modules directory in the pipeline modules ([str]): List of names of installed modules from the repository Returns: @@ -245,7 +245,7 @@ def determine_module_branches_and_shas(self, repo_name, remote_url, modules): {"name": "Remove the files", "value": 1}, ], style=nf_core.utils.nfcore_question_style, - ) + ).unsafe_ask() if action == 0: sb_local.append(module) else: @@ -320,10 +320,9 @@ def unsynced_modules(self): directories containing a 'main.nf' file Returns: - (untrack_dirs ([ Path ]), - missing_installation (dict)): Directories that are not tracked - by the modules.json file, and modules in the modules.json that - where the installation directory is missing + (untrack_dirs ([ Path ]), missing_installation (dict)): Directories that are not tracked + by the modules.json file, and modules in the modules.json where + the installation directory is missing """ missing_installation = copy.deepcopy(self.modules_json["repos"]) dirs = [ @@ -342,19 +341,19 @@ def unsynced_modules(self): if module_repo_name is not None: # If it does, check if the module is in the 'modules.json' file module = str(dir.relative_to(module_repo_name)) + module_repo = missing_installation[module_repo_name] - if module not in missing_installation[module_repo_name].get("modules", {}): + if module not in module_repo.get("modules", {}): untracked_dirs.append(dir) else: # Check if the entry has a git sha and branch before removing - modules = missing_installation[module_repo_name]["modules"] + modules = module_repo["modules"] if "git_sha" not in modules[module] or "branch" not in modules[module]: - raise UserWarning( - "The 'modules.json' file is not up to date. " - "Please reinstall it by removing it and rerunning the command." + self.determine_module_branches_and_shas( + module, module_repo["git_url"], module_repo["base_path"], [module] ) - missing_installation[module_repo_name]["modules"].pop(module) - if len(missing_installation[module_repo_name]["modules"]) == 0: + module_repo["modules"].pop(module) + if len(module_repo["modules"]) == 0: missing_installation.pop(module_repo_name) else: # If it is not, add it to the list of missing modules @@ -362,16 +361,24 @@ def unsynced_modules(self): return untracked_dirs, missing_installation - def has_git_url(self): + def has_git_url_and_modules(self): """ - Check that that all repo entries in the modules.json - has a git url - + Check that all repo entries in the modules.json + has a git url and a modules dict entry Returns: (bool): True if they are found for all repos, False otherwise """ for repo_entry in self.modules_json.get("repos", {}).values(): - if "git_url" not in repo_entry: + if "git_url" not in repo_entry or "modules" not in repo_entry: + log.warning(f"modules.json entry {repo_entry} does not have a git_url or modules entry") + return False + elif ( + not isinstance(repo_entry["git_url"], str) + or repo_entry["git_url"] == "" + or not isinstance(repo_entry["modules"], dict) + or repo_entry["modules"] == {} + ): + log.warning(f"modules.json entry {repo_entry} has non-string or empty entries for git_url or modules") return False return True @@ -424,12 +431,13 @@ def check_up_to_date(self): If a module is installed but the entry in 'modules.json' is missing we iterate through the commit log in the remote to try to determine the SHA. """ - self.load() - if not self.has_git_url(): - raise UserWarning( - "The 'modules.json' file is not up to date. " - "Please reinstall it by removing it and rerunning the command." - ) + try: + self.load() + if not self.has_git_url_and_modules(): + raise UserWarning + except UserWarning: + log.info("The 'modules.json' file is not up to date. Recreating the 'module.json' file.") + self.create() missing_from_modules_json, missing_installation = self.unsynced_modules() diff --git a/nf_core/modules/remove.py b/nf_core/modules/remove.py index 7145a5fac9..3a248bd647 100644 --- a/nf_core/modules/remove.py +++ b/nf_core/modules/remove.py @@ -21,8 +21,9 @@ def remove(self, module): log.error("You cannot remove a module in a clone of nf-core/modules") return False - # Check whether pipelines is valid + # Check whether pipeline is valid and with a modules.json file self.has_valid_directory() + self.has_modules_file() repo_name = self.modules_repo.fullname if module is None: diff --git a/tests/modules/modules_json.py b/tests/modules/modules_json.py index 7e9d211f3b..2412f9bd2d 100644 --- a/tests/modules/modules_json.py +++ b/tests/modules/modules_json.py @@ -1,3 +1,4 @@ +import copy import json import os import shutil @@ -216,3 +217,33 @@ def test_mod_json_dump(self): with open(mod_json_path, "r") as f: mod_json_new = json.load(f) assert mod_json == mod_json_new + + +def test_mod_json_with_empty_modules_value(self): + # Load module.json and remove the modules entry + mod_json_obj = ModulesJson(self.pipeline_dir) + mod_json_orig = mod_json_obj.get_modules_json() + mod_json = copy.deepcopy(mod_json_orig) + mod_json["repos"]["nf-core/modules"]["modules"] = "" + # save the altered module.json and load it again to check if it will fix itself + mod_json_obj.modules_json = mod_json + mod_json_obj.dump() + mod_json_obj_new = ModulesJson(self.pipeline_dir) + mod_json_obj_new.check_up_to_date() + mod_json_new = mod_json_obj_new.get_modules_json() + assert mod_json_orig == mod_json_new + + +def test_mod_json_with_missing_modules_entry(self): + # Load module.json and remove the modules entry + mod_json_obj = ModulesJson(self.pipeline_dir) + mod_json_orig = mod_json_obj.get_modules_json() + mod_json = copy.deepcopy(mod_json_orig) + mod_json["repos"]["nf-core/modules"].pop("modules") + # save the altered module.json and load it again to check if it will fix itself + mod_json_obj.modules_json = mod_json + mod_json_obj.dump() + mod_json_obj_new = ModulesJson(self.pipeline_dir) + mod_json_obj_new.check_up_to_date() + mod_json_new = mod_json_obj_new.get_modules_json() + assert mod_json_orig == mod_json_new diff --git a/tests/test_modules.py b/tests/test_modules.py index 3b854a8e3c..cf8c1c82f6 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -141,6 +141,8 @@ def test_modulesrepo_class(self): test_mod_json_up_to_date_module_removed, test_mod_json_up_to_date_reinstall_fails, test_mod_json_update, + test_mod_json_with_empty_modules_value, + test_mod_json_with_missing_modules_entry, ) from .modules.patch import ( test_create_patch_change,