From 6e5b7a3f950269c47878aa3929fef49fec4ec16f Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 28 Apr 2022 14:31:26 +0200 Subject: [PATCH 01/93] remove unused import --- nf_core/modules/update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 1967ac55a4..577acb0f95 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -7,7 +7,6 @@ import questionary import shutil import tempfile -from questionary import question from rich.console import Console from rich.syntax import Syntax From d5c111204df27e96c6a5a57ce8991bf507779962 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 28 Apr 2022 14:34:38 +0200 Subject: [PATCH 02/93] use early return pattern --- nf_core/modules/update.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 577acb0f95..dd242d0641 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -105,20 +105,20 @@ def update(self, module): if config_entry is False: log.info("Module's update entry in '.nf-core.yml' is set to False") return False - elif isinstance(config_entry, str): - sha = config_entry - if self.sha: - log.warning( - f"Found entry in '.nf-core.yml' for module '{module}' " - "which will override version specified with '--sha'" - ) - else: - log.info(f"Found entry in '.nf-core.yml' for module '{module}'") - log.info(f"Updating module to ({sha})") - else: + if not isinstance(config_entry, str): log.error("Module's update entry in '.nf-core.yml' is of wrong type") return False + sha = config_entry + if self.sha: + log.warning( + f"Found entry in '.nf-core.yml' for module '{module}' " + "which will override version specified with '--sha'" + ) + else: + log.info(f"Found entry in '.nf-core.yml' for module '{module}'") + log.info(f"Updating module to ({sha})") + # Check that the supplied name is an available module if module and module not in self.modules_repo.modules_avail_module_names: log.error("Module '{}' not found in list of available modules.".format(module)) From 789b76d1ed1d9772b98c15f521459662bd07240b Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 28 Apr 2022 14:36:39 +0200 Subject: [PATCH 03/93] bug fix: variable name --- nf_core/modules/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index dd242d0641..476c1f9f5c 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -190,7 +190,7 @@ def update(self, module): # If --preview is true, don't save to a patch file if self.show_diff: - self.show_diff_fn = False + self.save_diff_fn = False # Ask if we should show the diffs (unless a filename was already given on the command line) if not self.save_diff_fn and self.show_diff is None: From e753d9fd86a398d329a4bbacd893dc8074aeae79 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 28 Apr 2022 14:42:30 +0200 Subject: [PATCH 04/93] use f string instead of format --- nf_core/modules/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 476c1f9f5c..63eeecc1fa 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -121,7 +121,7 @@ def update(self, module): # Check that the supplied name is an available module if module and module not in self.modules_repo.modules_avail_module_names: - log.error("Module '{}' not found in list of available modules.".format(module)) + log.error(f"Module '{module}' not found in list of available modules.") log.info("Use the command 'nf-core modules list remote' to view available software") return False From 5764eadaa95ef26353606ba23c75a91b54c156b3 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 28 Apr 2022 14:45:08 +0200 Subject: [PATCH 05/93] do not use f-strings unnecessarily --- nf_core/modules/update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 63eeecc1fa..997d9f2d97 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -220,7 +220,7 @@ def update(self, module): os.remove(self.save_diff_fn) break self.save_diff_fn = questionary.text( - f"Enter a new filename: ", + "Enter a new filename: ", style=nf_core.utils.nfcore_question_style, ).unsafe_ask() @@ -444,7 +444,7 @@ class DiffEnum(enum.Enum): # Save diff for modules.json to file with open(self.save_diff_fn, "a") as fh: - fh.write(f"Changes in './modules.json'\n") + fh.write("Changes in './modules.json'\n") for line in modules_json_diff: fh.write(line) fh.write("*" * 60 + "\n") From e3d24b3f5bed1309ea2807226ef24c1f3c601495 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:23:55 +0200 Subject: [PATCH 06/93] use nested unpacking --- nf_core/modules/update.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 997d9f2d97..d57bd5c477 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -360,8 +360,7 @@ class DiffEnum(enum.Enum): f"Changes in module '{module}' between ({current_entry['git_sha'] if current_entry is not None else '?'}) and ({version if version is not None else 'latest'})\n" ) - for file, d in diffs.items(): - diff_status, diff = d + for file, (diff_status, diff) in diffs.items(): if diff_status == DiffEnum.UNCHANGED: # The files are identical fh.write(f"'{os.path.join(module_dir, file)}' is unchanged\n") @@ -389,8 +388,7 @@ class DiffEnum(enum.Enum): f"Changes in module '{module}' between ({current_entry['git_sha'] if current_entry is not None else '?'}) and ({version if version is not None else 'latest'})" ) - for file, d in diffs.items(): - diff_status, diff = d + for file, (diff_status, diff) in diffs.items(): if diff_status == DiffEnum.UNCHANGED: # The files are identical log.info(f"'{os.path.join(module, file)}' is unchanged") From 2491a03fc26f0174051834489e110c259cf87865 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:31:29 +0200 Subject: [PATCH 07/93] wrapping f-strings --- nf_core/modules/update.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index d57bd5c477..90bef15ec3 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -357,7 +357,9 @@ class DiffEnum(enum.Enum): log.info(f"Writing diff of '{module}' to '{self.save_diff_fn}'") with open(self.save_diff_fn, "a") as fh: fh.write( - f"Changes in module '{module}' between ({current_entry['git_sha'] if current_entry is not None else '?'}) and ({version if version is not None else 'latest'})\n" + f"Changes in module '{module}' between" + f" ({current_entry['git_sha'] if current_entry is not None else '?'}) and" + f" ({version if version is not None else 'latest'})\n" ) for file, (diff_status, diff) in diffs.items(): @@ -385,7 +387,9 @@ class DiffEnum(enum.Enum): elif self.show_diff: console = Console(force_terminal=nf_core.utils.rich_force_colors()) log.info( - f"Changes in module '{module}' between ({current_entry['git_sha'] if current_entry is not None else '?'}) and ({version if version is not None else 'latest'})" + f"Changes in module '{module}' between" + f" ({current_entry['git_sha'] if current_entry is not None else '?'}) and" + f" ({version if version is not None else 'latest'})" ) for file, (diff_status, diff) in diffs.items(): @@ -451,7 +455,9 @@ class DiffEnum(enum.Enum): if self.save_diff_fn: log.info( - f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you can apply them by running the command :point_right: [bold magenta italic]git apply {self.save_diff_fn}" + f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " + "can apply them by running the command :point_right:" + f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) return exit_value From af726e68cca7fc4b8e067e0a075d56b179d713f9 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:33:07 +0200 Subject: [PATCH 08/93] say updates complete only after updates --- nf_core/modules/update.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 90bef15ec3..be0392047e 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -451,13 +451,14 @@ class DiffEnum(enum.Enum): fh.write(line) fh.write("*" * 60 + "\n") - log.info("Updates complete :sparkles:") - - if self.save_diff_fn: log.info( f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " "can apply them by running the command :point_right:" f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) + else: + + log.info("Updates complete :sparkles:") + return exit_value From 7f892f2682a4268eff7799ae735d61e0cb0ad99b Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:43:05 +0200 Subject: [PATCH 09/93] use a list comprehension --- nf_core/modules/update.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index be0392047e..cae53ce056 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -139,9 +139,7 @@ def update(self, module): repos_mods_shas = {} for repo_name, modules in self.module_names.items(): if repo_name not in update_config or update_config[repo_name] is True: - repos_mods_shas[repo_name] = [] - for module in modules: - repos_mods_shas[repo_name].append((module, self.sha)) + repos_mods_shas[repo_name] = [(module, self.sha) for module in modules] elif isinstance(update_config[repo_name], dict): repo_config = update_config[repo_name] repos_mods_shas[repo_name] = [] From 0c2089bf977a89a57654f69c3b7c86b9790f86ab Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:43:25 +0200 Subject: [PATCH 10/93] fail if the arguments are incompatible --- nf_core/modules/update.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index cae53ce056..ddbae21056 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -32,7 +32,21 @@ def __init__( self.show_diff = show_diff self.save_diff_fn = save_diff_fn + def _parameter_compatibility_check(self): + """Check the compatibilty of the supplied parameters. + + Checks: + - Either `--preview` or `--save_diff` can be specified, not both. + """ + + # Can't this be handled by click with mutually exclusive parameters? + if self.save_diff_fn and self.show_diff: + raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") + def update(self, module): + + self._parameter_compatibility_check() + if self.repo_type == "modules": log.error("You cannot update a module in a clone of nf-core/modules") return False @@ -186,10 +200,6 @@ def update(self, module): if not modules_json: return False - # If --preview is true, don't save to a patch file - if self.show_diff: - self.save_diff_fn = False - # Ask if we should show the diffs (unless a filename was already given on the command line) if not self.save_diff_fn and self.show_diff is None: diff_type = questionary.select( From 81a114771f63bf2e8512ede5317abcb43d629536 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 10:48:38 +0200 Subject: [PATCH 11/93] adopt isort --- nf_core/modules/update.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index ddbae21056..756f83cc20 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -4,17 +4,20 @@ import json import logging import os -import questionary import shutil import tempfile + +import questionary from rich.console import Console from rich.syntax import Syntax -import nf_core.utils import nf_core.modules.module_utils +import nf_core.utils +from .module_utils import get_installed_modules +from .module_utils import get_module_git_log +from .module_utils import module_exist_in_repo from .modules_command import ModuleCommand -from .module_utils import get_installed_modules, get_module_git_log, module_exist_in_repo from .modules_repo import ModulesRepo log = logging.getLogger(__name__) From 64d27e7c17cc2aefd12a8bd3e94a929dc7e42bb5 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 11:18:32 +0200 Subject: [PATCH 12/93] remove unused import --- nf_core/modules/update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 756f83cc20..ec2dd875bb 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -14,7 +14,6 @@ import nf_core.modules.module_utils import nf_core.utils -from .module_utils import get_installed_modules from .module_utils import get_module_git_log from .module_utils import module_exist_in_repo from .modules_command import ModuleCommand From 0b58244db1f9882862f4fd95f375dc6d297084a6 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 13:24:18 +0200 Subject: [PATCH 13/93] use import from --- nf_core/modules/update.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index ec2dd875bb..9a8cc4512a 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -16,6 +16,7 @@ from .module_utils import get_module_git_log from .module_utils import module_exist_in_repo +from .module_utils import sha_exists from .modules_command import ModuleCommand from .modules_repo import ModulesRepo @@ -79,7 +80,7 @@ def update(self, module): # Verify that the provided SHA exists in the repo if self.sha: try: - nf_core.modules.module_utils.sha_exists(self.sha, self.modules_repo) + sha_exists(self.sha, self.modules_repo) except UserWarning: log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.name}'") return False From 4293b0c9749166463c38c0daa5284a888e10a8e7 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 13:25:05 +0200 Subject: [PATCH 14/93] use list comprehension --- nf_core/modules/update.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 9a8cc4512a..e6a8ab67b3 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -173,9 +173,7 @@ def update(self, module): elif isinstance(update_config[repo_name], str): # If a string is given it is the commit SHA to which we should update to custom_sha = update_config[repo_name] - repos_mods_shas[repo_name] = [] - for module in modules: - repos_mods_shas[repo_name].append((module, custom_sha)) + repos_mods_shas[repo_name] = [(module_name, custom_sha) for module_name in modules] else: skipped_repos.append(repo_name) From b170827c5310f82aa6957d967b4711f0ff9f18c7 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 15:20:29 +0200 Subject: [PATCH 15/93] let tempfile chose a temporary directory --- nf_core/modules/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index e6a8ab67b3..78453077f1 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -301,7 +301,7 @@ def update(self, module): if dry_run: # Set the install folder to a temporary directory - install_folder = ["/tmp", next(tempfile._get_candidate_names())] + install_folder = [tempfile.mkdtemp()] # Download module files if not self.download_module_file(module, version, modules_repo, install_folder, dry_run=dry_run): From e9e92f283f8c0ca2cff2f36340741ee87cd14fea Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 15:23:52 +0200 Subject: [PATCH 16/93] collect parameter test prompt+sha --- nf_core/modules/update.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 78453077f1..af2241f572 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -59,6 +59,8 @@ def update(self, module): # Verify that 'modules.json' is consistent with the installed modules self.modules_json_up_to_date() + if self.prompt and self.sha is not None: + raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") tool_config = nf_core.utils.load_tools_config() update_config = tool_config.get("update", {}) @@ -73,10 +75,6 @@ def update(self, module): == "All modules" ) - if self.prompt and self.sha is not None: - log.error("Cannot use '--sha' and '--prompt' at the same time!") - return False - # Verify that the provided SHA exists in the repo if self.sha: try: From 89a115b1a266a0a744bd375c6f35d5a4e082915c Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 15:26:12 +0200 Subject: [PATCH 17/93] collect parameter test module+all --- nf_core/modules/update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index af2241f572..ace86c4785 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -49,6 +49,8 @@ def _parameter_compatibility_check(self): def update(self, module): self._parameter_compatibility_check() + if self.update_all and self.module: + raise UserWarning("Either a module or the '--all' flag can be specified, not both.") if self.repo_type == "modules": log.error("You cannot update a module in a clone of nf-core/modules") @@ -143,8 +145,6 @@ def update(self, module): repos_mods_shas = [(self.modules_repo, module, sha)] else: - if module: - raise UserWarning("You cannot specify a module and use the '--all' flag at the same time") self.get_pipeline_modules() From 2e44600e56d5cb8b608c4c835765986cc3831468 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 15:28:29 +0200 Subject: [PATCH 18/93] raise UserWarning instead of return --- nf_core/modules/update.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index ace86c4785..a3d619688e 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -53,11 +53,10 @@ def update(self, module): raise UserWarning("Either a module or the '--all' flag can be specified, not both.") if self.repo_type == "modules": - log.error("You cannot update a module in a clone of nf-core/modules") - return False - # Check whether pipelines is valid + raise UserWarning("Modules in clones of nf-core/modules can not be updated.") + if not self.has_valid_directory(): - return False + raise UserWarning("The command was not run in a valid pipeline directory.") # Verify that 'modules.json' is consistent with the installed modules self.modules_json_up_to_date() From 6e0ab52b1210a7e83e6f72a67f98bcf4d4ae009b Mon Sep 17 00:00:00 2001 From: fabianegli Date: Fri, 29 Apr 2022 15:33:12 +0200 Subject: [PATCH 19/93] remove unused import --- nf_core/modules/update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index a3d619688e..56783d68ce 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -11,7 +11,6 @@ from rich.console import Console from rich.syntax import Syntax -import nf_core.modules.module_utils import nf_core.utils from .module_utils import get_module_git_log From de5dcb7d178b5928a9b0a087ee58ee09140058ad Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 5 May 2022 09:57:13 +0200 Subject: [PATCH 20/93] click doesn't handle mutually exclusive flags --- nf_core/modules/update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 56783d68ce..b6c5a99c36 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -41,7 +41,6 @@ def _parameter_compatibility_check(self): - Either `--preview` or `--save_diff` can be specified, not both. """ - # Can't this be handled by click with mutually exclusive parameters? if self.save_diff_fn and self.show_diff: raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") From 19f3728bd48218c5f76600a7bd071904ec114128 Mon Sep 17 00:00:00 2001 From: Harshil Patel Date: Thu, 21 Apr 2022 11:32:30 +0100 Subject: [PATCH 21/93] Fix #1419 --- CHANGELOG.md | 1 + nf_core/schema.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86a8206b91..70bd3e838f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Add an empty line to `modules.json`, `params.json` and `nextflow-schema.json` when dumping them to avoid prettier errors. - Add actions workflow to respond to `@nf-core-bot fix linting` comments on nf-core/tools PRs - Linting: Don't allow a `.nf-core.yaml` file, should be `.yml` ([#1515](https://github.com/nf-core/tools/pull/1515)). +- Not all definitions in JSON schema have a "properties", leading to an error ([#1419](https://github.com/nf-core/tools/issues/1419)) ### Modules diff --git a/nf_core/schema.py b/nf_core/schema.py index 5784645543..522f48d20b 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -613,6 +613,29 @@ def get_wf_params(self): ) ) + def remove_schema_empty_definitions(self): + """ + Go through top-level schema remove definitions that don't have + any property attributes + """ + # Make copy of schema + schema_no_empty_definitions = copy.deepcopy(self.schema) + + ## Identify and remove empty definitions from the schema + empty_definitions = [] + for d_key, d_schema in list(schema_no_empty_definitions.get("definitions", {}).items()): + if not d_schema['properties']: + del schema_no_empty_definitions["definitions"][d_key] + empty_definitions.append(d_key) + + # Remove "allOf" group with empty definitions from the schema + for d_key in empty_definitions: + allOf = {'$ref': "#/definitions/{}".format(d_key)} + if allOf in schema_no_empty_definitions["allOf"]: + schema_no_empty_definitions["allOf"].remove(allOf) + + self.schema = schema_no_empty_definitions + def remove_schema_notfound_configs(self): """ Go through top-level schema and all definitions sub-schemas to remove @@ -625,6 +648,8 @@ def remove_schema_notfound_configs(self): cleaned_schema, p_removed = self.remove_schema_notfound_configs_single_schema(definition) self.schema["definitions"][d_key] = cleaned_schema params_removed.extend(p_removed) + self.remove_schema_empty_definitions() + return params_removed def remove_schema_notfound_configs_single_schema(self, schema): From 36e546939e802d4cca30c408ef44ff359a2e3382 Mon Sep 17 00:00:00 2001 From: Harshil Patel Date: Thu, 21 Apr 2022 12:07:52 +0100 Subject: [PATCH 22/93] Black --- nf_core/schema.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/schema.py b/nf_core/schema.py index 522f48d20b..788adf73e2 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -624,13 +624,13 @@ def remove_schema_empty_definitions(self): ## Identify and remove empty definitions from the schema empty_definitions = [] for d_key, d_schema in list(schema_no_empty_definitions.get("definitions", {}).items()): - if not d_schema['properties']: + if not d_schema["properties"]: del schema_no_empty_definitions["definitions"][d_key] empty_definitions.append(d_key) # Remove "allOf" group with empty definitions from the schema for d_key in empty_definitions: - allOf = {'$ref': "#/definitions/{}".format(d_key)} + allOf = {"$ref": "#/definitions/{}".format(d_key)} if allOf in schema_no_empty_definitions["allOf"]: schema_no_empty_definitions["allOf"].remove(allOf) From e100be09b63d493efc2d890a5901f30545a8b986 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Thu, 5 May 2022 10:01:39 +0200 Subject: [PATCH 23/93] Handle missing key, log, call after web builder --- nf_core/schema.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nf_core/schema.py b/nf_core/schema.py index 788adf73e2..fcda38b200 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -536,6 +536,7 @@ def build_schema(self, pipeline_dir, no_prompts, web_only, url): self.get_wf_params() self.make_skeleton_schema() self.remove_schema_notfound_configs() + self.remove_schema_empty_definitions() self.add_schema_found_configs() try: self.validate_schema() @@ -555,6 +556,7 @@ def build_schema(self, pipeline_dir, no_prompts, web_only, url): if not self.web_only: self.get_wf_params() self.remove_schema_notfound_configs() + self.remove_schema_empty_definitions() self.add_schema_found_configs() self.save_schema() @@ -624,10 +626,13 @@ def remove_schema_empty_definitions(self): ## Identify and remove empty definitions from the schema empty_definitions = [] for d_key, d_schema in list(schema_no_empty_definitions.get("definitions", {}).items()): - if not d_schema["properties"]: + if not d_schema.get("properties"): del schema_no_empty_definitions["definitions"][d_key] empty_definitions.append(d_key) + if len(empty_definitions): + log.warning(f"Removing empty group: '{', '.join(empty_definitions)}'") + # Remove "allOf" group with empty definitions from the schema for d_key in empty_definitions: allOf = {"$ref": "#/definitions/{}".format(d_key)} @@ -648,7 +653,6 @@ def remove_schema_notfound_configs(self): cleaned_schema, p_removed = self.remove_schema_notfound_configs_single_schema(definition) self.schema["definitions"][d_key] = cleaned_schema params_removed.extend(p_removed) - self.remove_schema_empty_definitions() return params_removed @@ -797,6 +801,7 @@ def get_web_builder_response(self): log.info("Found saved status from nf-core schema builder") try: self.schema = web_response["schema"] + self.remove_schema_empty_definitions() self.validate_schema() except AssertionError as e: raise AssertionError("Response from schema builder did not pass validation:\n {}".format(e)) From 143c041a54eb03a3fb33ee6e52f2cabea1f620ff Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Thu, 5 May 2022 10:21:33 +0200 Subject: [PATCH 24/93] Deep copy of schema not needed --- nf_core/schema.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/nf_core/schema.py b/nf_core/schema.py index fcda38b200..05706d0dd1 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -620,26 +620,19 @@ def remove_schema_empty_definitions(self): Go through top-level schema remove definitions that don't have any property attributes """ - # Make copy of schema - schema_no_empty_definitions = copy.deepcopy(self.schema) - - ## Identify and remove empty definitions from the schema + # Identify and remove empty definitions from the schema empty_definitions = [] - for d_key, d_schema in list(schema_no_empty_definitions.get("definitions", {}).items()): + for d_key, d_schema in list(self.schema.get("definitions", {}).items()): if not d_schema.get("properties"): - del schema_no_empty_definitions["definitions"][d_key] + del self.schema["definitions"][d_key] empty_definitions.append(d_key) - - if len(empty_definitions): - log.warning(f"Removing empty group: '{', '.join(empty_definitions)}'") + log.warning(f"Removing empty group: '{d_key}'") # Remove "allOf" group with empty definitions from the schema for d_key in empty_definitions: allOf = {"$ref": "#/definitions/{}".format(d_key)} - if allOf in schema_no_empty_definitions["allOf"]: - schema_no_empty_definitions["allOf"].remove(allOf) - - self.schema = schema_no_empty_definitions + if allOf in self.schema.get("allOf", []): + self.schema["allOf"].remove(allOf) def remove_schema_notfound_configs(self): """ From 4cc3b045a1b2c4dc15a81bbe2d97b5816fdfdb07 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Thu, 5 May 2022 10:22:13 +0200 Subject: [PATCH 25/93] Format strings --- nf_core/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/schema.py b/nf_core/schema.py index 05706d0dd1..20be305f99 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -630,7 +630,7 @@ def remove_schema_empty_definitions(self): # Remove "allOf" group with empty definitions from the schema for d_key in empty_definitions: - allOf = {"$ref": "#/definitions/{}".format(d_key)} + allOf = {"$ref": f"#/definitions/{d_key}"} if allOf in self.schema.get("allOf", []): self.schema["allOf"].remove(allOf) From 5edb359a14b27957de56df7763d31509ebae02a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Thu, 21 Apr 2022 10:35:36 +0200 Subject: [PATCH 26/93] print include statement when installing a module --- nf_core/modules/install.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nf_core/modules/install.py b/nf_core/modules/install.py index 673e1d7f4e..a72c45da76 100644 --- a/nf_core/modules/install.py +++ b/nf_core/modules/install.py @@ -130,6 +130,14 @@ def install(self, module): if not self.download_module_file(module, version, self.modules_repo, install_folder): return False + # Print include statement + if module == "stringtie/stringtie": + # Only with stringtie the process name is STRINGTIE instead of STRINGTIE_STRINGTIE + module_name = module.upper().split("/")[0] + else: + module_name = "_".join(module.upper().split("/")) + log.info(f"Include statement: include {{ {module_name} }} from '.{os.path.join(*install_folder, module)}/main’") + # Update module.json with newly installed module self.update_modules_json(modules_json, self.modules_repo.name, module, version) return True From b0794f78edf84b0254684a38f8fa8052e15f9aa7 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 12 May 2022 09:25:45 +0200 Subject: [PATCH 27/93] resolved rebase merge conflict --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70bd3e838f..2b4c6c0c97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Removed retry strategy for AWS tests CI, as Nextflow now handles spot instance retries itself - Add `.prettierignore` file to stop Prettier linting tests from running over test files - Add actions workflow to respond to `@nf-core-bot fix linting` comments on pipeline PRs +- Print include statement to terminal when `modules install` ([#1520](https://github.com/nf-core/tools/pull/1520)) ### General From dccef6c3aa183be7cef373f295afcaee38069e3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Thu, 21 Apr 2022 13:52:56 +0200 Subject: [PATCH 28/93] remove remove stringtie special case --- nf_core/modules/install.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nf_core/modules/install.py b/nf_core/modules/install.py index a72c45da76..96f04cb34c 100644 --- a/nf_core/modules/install.py +++ b/nf_core/modules/install.py @@ -131,11 +131,7 @@ def install(self, module): return False # Print include statement - if module == "stringtie/stringtie": - # Only with stringtie the process name is STRINGTIE instead of STRINGTIE_STRINGTIE - module_name = module.upper().split("/")[0] - else: - module_name = "_".join(module.upper().split("/")) + module_name = "_".join(module.upper().split("/")) log.info(f"Include statement: include {{ {module_name} }} from '.{os.path.join(*install_folder, module)}/main’") # Update module.json with newly installed module From 253626679ad394ec3b35f95fef8ea6a8c7d595b6 Mon Sep 17 00:00:00 2001 From: Nathan Spix <56930974+njspix@users.noreply.github.com> Date: Tue, 26 Apr 2022 09:29:15 -0400 Subject: [PATCH 29/93] Update actions_ci.py In cases where e.g. multiple aligners are specified in the GitHub Actions matrix, trying to pull a 'NXF_VER' key out of the include dictionary can cause line 140 to fail when it shouldn't. The get() method will not error if the key is not present, fixing this problem. --- nf_core/lint/actions_ci.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/lint/actions_ci.py b/nf_core/lint/actions_ci.py index 89d9becd04..1a02680ece 100644 --- a/nf_core/lint/actions_ci.py +++ b/nf_core/lint/actions_ci.py @@ -137,7 +137,7 @@ def actions_ci(self): # Check that we are testing the minimum nextflow version try: matrix = ciwf["jobs"]["test"]["strategy"]["matrix"]["include"] - assert any([i["NXF_VER"] == self.minNextflowVersion for i in matrix]) + assert any([i.get("NXF_VER") == self.minNextflowVersion for i in matrix]) except (KeyError, TypeError): failed.append("'.github/workflows/ci.yml' does not check minimum NF version") except AssertionError: From 1876107c127f0611d311ed355b5d15064ec166da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 25 Apr 2022 11:40:08 +0200 Subject: [PATCH 30/93] resolve rebase mergeconflicts --- nf_core/__main__.py | 20 ++++++++- nf_core/modules/__init__.py | 1 + nf_core/modules/module_test.py | 81 ++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 nf_core/modules/module_test.py diff --git a/nf_core/__main__.py b/nf_core/__main__.py index ae8b995c99..d2bfcd78e1 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -48,7 +48,7 @@ }, { "name": "Developing new modules", - "commands": ["create", "create-test-yml", "lint", "bump-versions", "mulled"], + "commands": ["create", "create-test-yml", "lint", "bump-versions", "mulled", "test"], }, ], } @@ -706,6 +706,24 @@ def mulled(specifications, build_number): "If it does not, please add your desired combination as detailed at:\n" "https://github.com/BioContainers/multi-package-containers\n" ) + + +# nf-core modules test +@modules.command("test") +@click.pass_context +@click.argument("tool", type=str, required=False, metavar=" or ") +@click.option("-p", "--no-prompts", is_flag=True, default=False, help="Use defaults without prompting") +def test_module(ctx, tool, no_prompts): + """ + Run module tests locally. + + Given the name of a module, runs the Nextflow test command. + """ + try: + meta_builder = nf_core.modules.ModulesTest(tool, no_prompts) + meta_builder.run() + except UserWarning as e: + log.critical(e) sys.exit(1) diff --git a/nf_core/modules/__init__.py b/nf_core/modules/__init__.py index 6b999aea50..29110461b5 100644 --- a/nf_core/modules/__init__.py +++ b/nf_core/modules/__init__.py @@ -10,3 +10,4 @@ from .remove import ModuleRemove from .info import ModuleInfo from .mulled import MulledImageNameGenerator +from .module_test import ModulesTest diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py new file mode 100644 index 0000000000..4d9bf95014 --- /dev/null +++ b/nf_core/modules/module_test.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python +""" +The ModulesTest class runs the tests locally +""" + +import logging +import questionary +import os + +"""from __future__ import print_function +from rich.syntax import Syntax + +import errno +import gzip +import hashlib +import operator + + +import re +import rich +import shlex +import subprocess +import tempfile +import yaml + +""" +import nf_core.utils +from .modules_repo import ModulesRepo +from .test_yml_builder import ModulesTestYmlBuilder + +log = logging.getLogger(__name__) + + +class ModulesTest(ModulesTestYmlBuilder): + def __init__( + self, + module_name=None, + no_prompts=False, + ): + self.run_tests = True + self.module_name = module_name + self.no_prompts = no_prompts + self.module_dir = None + self.module_test_main = None + self.entry_points = [] + self.tests = [] + self.errors = [] + + def run(self): + """Run test steps""" + if not self.no_prompts: + log.info( + "[yellow]Press enter to use default values [cyan bold](shown in brackets) [yellow]or type your own responses" + ) + self.check_inputs_test() + self.scrape_workflow_entry_points() + self.build_all_tests() + if len(self.errors) > 0: + errors = "\n - ".join(self.errors) + raise UserWarning(f"Ran, but found errors:\n - {errors}") + + def check_inputs_test(self): + """Do more complex checks about supplied flags.""" + + # Get the tool name if not specified + if self.module_name is None: + modules_repo = ModulesRepo() + modules_repo.get_modules_file_tree() + self.module_name = questionary.autocomplete( + "Tool name:", + choices=modules_repo.modules_avail_module_names, + style=nf_core.utils.nfcore_question_style, + ).ask() + self.module_dir = os.path.join("modules", *self.module_name.split("/")) + self.module_test_main = os.path.join("tests", "modules", *self.module_name.split("/"), "main.nf") + + # First, sanity check that the module directory exists + if not os.path.isdir(self.module_dir): + raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") + if not os.path.exists(self.module_test_main): + raise UserWarning(f"Cannot find module test workflow '{self.module_test_main}'") From 2e713dd54ab111caae975d7387fb04bba97e7e52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 10:18:15 +0200 Subject: [PATCH 31/93] run tests with pytest --- nf_core/__main__.py | 5 ++- nf_core/modules/module_test.py | 80 ++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index d2bfcd78e1..358b11a7b5 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -713,14 +713,15 @@ def mulled(specifications, build_number): @click.pass_context @click.argument("tool", type=str, required=False, metavar=" or ") @click.option("-p", "--no-prompts", is_flag=True, default=False, help="Use defaults without prompting") -def test_module(ctx, tool, no_prompts): +@click.option("-a", "--pytest_args", type=str, required=False, multiple=True, help="Additional pytest arguments") +def test_module(ctx, tool, no_prompts, pytest_args): """ Run module tests locally. Given the name of a module, runs the Nextflow test command. """ try: - meta_builder = nf_core.modules.ModulesTest(tool, no_prompts) + meta_builder = nf_core.modules.ModulesTest(tool, no_prompts, pytest_args) meta_builder.run() except UserWarning as e: log.critical(e) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 4d9bf95014..8316d9502d 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -6,45 +6,25 @@ import logging import questionary import os - -"""from __future__ import print_function -from rich.syntax import Syntax - -import errno -import gzip -import hashlib -import operator - - -import re +import pytest +import sys import rich -import shlex -import subprocess -import tempfile -import yaml -""" import nf_core.utils from .modules_repo import ModulesRepo -from .test_yml_builder import ModulesTestYmlBuilder log = logging.getLogger(__name__) - -class ModulesTest(ModulesTestYmlBuilder): +class ModulesTest(object): def __init__( self, module_name=None, no_prompts=False, + pytest_args="", ): - self.run_tests = True self.module_name = module_name self.no_prompts = no_prompts - self.module_dir = None - self.module_test_main = None - self.entry_points = [] - self.tests = [] - self.errors = [] + self.pytest_args = pytest_args def run(self): """Run test steps""" @@ -52,14 +32,11 @@ def run(self): log.info( "[yellow]Press enter to use default values [cyan bold](shown in brackets) [yellow]or type your own responses" ) - self.check_inputs_test() - self.scrape_workflow_entry_points() - self.build_all_tests() - if len(self.errors) > 0: - errors = "\n - ".join(self.errors) - raise UserWarning(f"Ran, but found errors:\n - {errors}") + self.check_inputs() + self.set_profile() + self.run_pytests() - def check_inputs_test(self): + def check_inputs(self): """Do more complex checks about supplied flags.""" # Get the tool name if not specified @@ -79,3 +56,42 @@ def check_inputs_test(self): raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") if not os.path.exists(self.module_test_main): raise UserWarning(f"Cannot find module test workflow '{self.module_test_main}'") + + def set_profile(self): + """Set $PROFILE env variable. + The config expects $PROFILE and Nextflow fails if it's not set. + """ + if os.environ.get("PROFILE") is None: + os.environ["PROFILE"] = "" + if self.no_prompts: + log.info( + "Setting env var '$PROFILE' to an empty string as not set.\n" + "Tests will run with Docker by default. " + "To use Singularity set 'export PROFILE=singularity' in your shell before running this command." + ) + else: + question = { + "type": "list", + "name": "profile", + "message": "Choose software profile", + "choices": ["Docker", "Singularity", "Conda"], + } + answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style) + profile = answer["profile"].lower() + if profile in ["singularity", "conda"]: + os.environ["PROFILE"] = profile + log.info(f"Setting env var '$PROFILE' to '{profile}'") + + def run_pytests(self): + """Given a module name, run tests.""" + # Print nice divider line + console = rich.console.Console() + console.print("[black]" + "─" * console.width) + + # Set pytest arguments + command_args = ["--tag", f"{self.module_name}", "--symlink", "--keep-workflow-wd"] + command_args += self.pytest_args + + # Run pytest + log.info(f"Running pytest for module '{self.module_name}'") + sys.exit(pytest.main(command_args)) From b55aecabcc9deb9c5d7d74f89ab7c78d83e80bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 10:29:36 +0200 Subject: [PATCH 32/93] remove test workflow check --- nf_core/modules/module_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 8316d9502d..ee5e39c4f0 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -25,6 +25,7 @@ def __init__( self.module_name = module_name self.no_prompts = no_prompts self.pytest_args = pytest_args + self.module_dir = None def run(self): """Run test steps""" @@ -49,13 +50,10 @@ def check_inputs(self): style=nf_core.utils.nfcore_question_style, ).ask() self.module_dir = os.path.join("modules", *self.module_name.split("/")) - self.module_test_main = os.path.join("tests", "modules", *self.module_name.split("/"), "main.nf") # First, sanity check that the module directory exists if not os.path.isdir(self.module_dir): raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") - if not os.path.exists(self.module_test_main): - raise UserWarning(f"Cannot find module test workflow '{self.module_test_main}'") def set_profile(self): """Set $PROFILE env variable. From 41e22326194eca29a7e3dafbad3b24ec2b5e58f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 13:10:10 +0200 Subject: [PATCH 33/93] add code tests --- tests/modules/module_test.py | 15 +++++++++++++++ tests/test_modules.py | 4 ++++ 2 files changed, 19 insertions(+) create mode 100644 tests/modules/module_test.py diff --git a/tests/modules/module_test.py b/tests/modules/module_test.py new file mode 100644 index 0000000000..12a4c85c4d --- /dev/null +++ b/tests/modules/module_test.py @@ -0,0 +1,15 @@ +"""Test the 'modules test' command which runs module pytests.""" +import os +import pytest + +import nf_core.modules + +def test_modules_test_check_inputs(self): + """Test the check_inputs() function - raise UserWarning because module doesn't exist""" + cwd = os.getcwd() + os.chdir(self.nfcore_modules) + meta_builder = nf_core.modules.ModulesTest("none", True, "") + with pytest.raises(UserWarning) as excinfo: + meta_builder.check_inputs() + os.chdir(cwd) + assert "Cannot find directory" in str(excinfo.value) diff --git a/tests/test_modules.py b/tests/test_modules.py index cfa3408e69..00cc89c36d 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -113,3 +113,7 @@ def test_modulesrepo_class(self): test_modules_bump_versions_fail, test_modules_bump_versions_fail_unknown_version, ) + + from .modules.module_test import ( + test_modules_test_check_inputs, + ) From 20dd418cf2b95f3becfe7429a091a6006c98d764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 13:29:42 +0200 Subject: [PATCH 34/93] fix linting --- nf_core/modules/module_test.py | 1 + tests/modules/module_test.py | 1 + 2 files changed, 2 insertions(+) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index ee5e39c4f0..5338aeb81e 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -15,6 +15,7 @@ log = logging.getLogger(__name__) + class ModulesTest(object): def __init__( self, diff --git a/tests/modules/module_test.py b/tests/modules/module_test.py index 12a4c85c4d..ac0c36c997 100644 --- a/tests/modules/module_test.py +++ b/tests/modules/module_test.py @@ -4,6 +4,7 @@ import nf_core.modules + def test_modules_test_check_inputs(self): """Test the check_inputs() function - raise UserWarning because module doesn't exist""" cwd = os.getcwd() From 362261588b9a0e742d42597307a1d1c4158fdb5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 15:33:13 +0200 Subject: [PATCH 35/93] apply comments from PR review --- nf_core/modules/module_test.py | 39 ++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 5338aeb81e..c6a81f8672 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -17,6 +17,31 @@ class ModulesTest(object): + """ + Class to run module pytests. + + ... + + Attributes + ---------- + module_name : str + name of the tool to run tests for + no_prompts : bool + flat indicating if prompts are used + pytest_args : tuple + additional arguments passed to pytest command + + Methods + ------- + run(): + Run test steps + __check_inputs(): + Check inputs. Ask for module_name if not provided and check that the directory exists + __set_profile(): + Set software profile + __run_pytests(self): + Run pytest + """ def __init__( self, module_name=None, @@ -34,15 +59,17 @@ def run(self): log.info( "[yellow]Press enter to use default values [cyan bold](shown in brackets) [yellow]or type your own responses" ) - self.check_inputs() - self.set_profile() - self.run_pytests() + self.__check_inputs() + self.__set_profile() + self.__run_pytests() - def check_inputs(self): + def __check_inputs(self): """Do more complex checks about supplied flags.""" # Get the tool name if not specified if self.module_name is None: + if self.no_prompts: + raise UserWarning(f"Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL.") modules_repo = ModulesRepo() modules_repo.get_modules_file_tree() self.module_name = questionary.autocomplete( @@ -56,7 +83,7 @@ def check_inputs(self): if not os.path.isdir(self.module_dir): raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") - def set_profile(self): + def __set_profile(self): """Set $PROFILE env variable. The config expects $PROFILE and Nextflow fails if it's not set. """ @@ -81,7 +108,7 @@ def set_profile(self): os.environ["PROFILE"] = profile log.info(f"Setting env var '$PROFILE' to '{profile}'") - def run_pytests(self): + def __run_pytests(self): """Given a module name, run tests.""" # Print nice divider line console = rich.console.Console() From d88a6cddf9af866ef5a1f4dbebba3945bac13fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 26 Apr 2022 15:50:51 +0200 Subject: [PATCH 36/93] fix linting --- nf_core/modules/module_test.py | 53 ++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index c6a81f8672..3d2aa8d58d 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -18,30 +18,31 @@ class ModulesTest(object): """ - Class to run module pytests. - - ... - - Attributes - ---------- - module_name : str - name of the tool to run tests for - no_prompts : bool - flat indicating if prompts are used - pytest_args : tuple - additional arguments passed to pytest command - - Methods - ------- - run(): - Run test steps - __check_inputs(): - Check inputs. Ask for module_name if not provided and check that the directory exists - __set_profile(): - Set software profile - __run_pytests(self): - Run pytest - """ + Class to run module pytests. + + ... + + Attributes + ---------- + module_name : str + name of the tool to run tests for + no_prompts : bool + flat indicating if prompts are used + pytest_args : tuple + additional arguments passed to pytest command + + Methods + ------- + run(): + Run test steps + __check_inputs(): + Check inputs. Ask for module_name if not provided and check that the directory exists + __set_profile(): + Set software profile + __run_pytests(self): + Run pytest + """ + def __init__( self, module_name=None, @@ -69,7 +70,9 @@ def __check_inputs(self): # Get the tool name if not specified if self.module_name is None: if self.no_prompts: - raise UserWarning(f"Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL.") + raise UserWarning( + f"Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL." + ) modules_repo = ModulesRepo() modules_repo.get_modules_file_tree() self.module_name = questionary.autocomplete( From 3f2a1de0effc00fb55084b60434cfcf8f155202e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 11:25:48 +0200 Subject: [PATCH 37/93] apply comments from PR --- nf_core/modules/module_test.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 3d2aa8d58d..bb39c27073 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -35,11 +35,11 @@ class ModulesTest(object): ------- run(): Run test steps - __check_inputs(): + _check_inputs(): Check inputs. Ask for module_name if not provided and check that the directory exists - __set_profile(): + _set_profile(): Set software profile - __run_pytests(self): + _run_pytests(self): Run pytest """ @@ -60,11 +60,11 @@ def run(self): log.info( "[yellow]Press enter to use default values [cyan bold](shown in brackets) [yellow]or type your own responses" ) - self.__check_inputs() - self.__set_profile() - self.__run_pytests() + self._check_inputs() + self._set_profile() + self._run_pytests() - def __check_inputs(self): + def _check_inputs(self): """Do more complex checks about supplied flags.""" # Get the tool name if not specified @@ -86,7 +86,7 @@ def __check_inputs(self): if not os.path.isdir(self.module_dir): raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") - def __set_profile(self): + def _set_profile(self): """Set $PROFILE env variable. The config expects $PROFILE and Nextflow fails if it's not set. """ @@ -111,7 +111,7 @@ def __set_profile(self): os.environ["PROFILE"] = profile log.info(f"Setting env var '$PROFILE' to '{profile}'") - def __run_pytests(self): + def _run_pytests(self): """Given a module name, run tests.""" # Print nice divider line console = rich.console.Console() From f3dc1875d61c4f7ef51019ec5c6f553d3faab265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 12:48:44 +0200 Subject: [PATCH 38/93] fix test --- tests/modules/module_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modules/module_test.py b/tests/modules/module_test.py index ac0c36c997..fbad1c9fc5 100644 --- a/tests/modules/module_test.py +++ b/tests/modules/module_test.py @@ -11,6 +11,6 @@ def test_modules_test_check_inputs(self): os.chdir(self.nfcore_modules) meta_builder = nf_core.modules.ModulesTest("none", True, "") with pytest.raises(UserWarning) as excinfo: - meta_builder.check_inputs() + meta_builder._check_inputs() os.chdir(cwd) assert "Cannot find directory" in str(excinfo.value) From e7fdd530a414eeac8a2cf8a241fdad715443d4f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 15:26:32 +0200 Subject: [PATCH 39/93] always set profile --- nf_core/modules/module_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index bb39c27073..7f33774776 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -107,9 +107,8 @@ def _set_profile(self): } answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style) profile = answer["profile"].lower() - if profile in ["singularity", "conda"]: - os.environ["PROFILE"] = profile - log.info(f"Setting env var '$PROFILE' to '{profile}'") + os.environ["PROFILE"] = profile + log.info(f"Setting env var '$PROFILE' to '{profile}'") def _run_pytests(self): """Given a module name, run tests.""" From 005143af228eb090b01f2402ad38b26165dfc79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 15:29:44 +0200 Subject: [PATCH 40/93] do not abbreviate env var --- nf_core/modules/module_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 7f33774776..db204ba97a 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -94,7 +94,7 @@ def _set_profile(self): os.environ["PROFILE"] = "" if self.no_prompts: log.info( - "Setting env var '$PROFILE' to an empty string as not set.\n" + "Setting environment variable '$PROFILE' to an empty string as not set.\n" "Tests will run with Docker by default. " "To use Singularity set 'export PROFILE=singularity' in your shell before running this command." ) @@ -108,7 +108,7 @@ def _set_profile(self): answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style) profile = answer["profile"].lower() os.environ["PROFILE"] = profile - log.info(f"Setting env var '$PROFILE' to '{profile}'") + log.info(f"Setting environment variable '$PROFILE' to '{profile}'") def _run_pytests(self): """Given a module name, run tests.""" From 9f48c9087eb6c162de49af9135b8c5924a3d6814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 16:06:53 +0200 Subject: [PATCH 41/93] add new test --- nf_core/modules/module_test.py | 2 +- tests/modules/module_test.py | 11 +++++++++++ tests/test_modules.py | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index db204ba97a..25e4991a4b 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -71,7 +71,7 @@ def _check_inputs(self): if self.module_name is None: if self.no_prompts: raise UserWarning( - f"Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL." + "Tool name not provided and prompts deactivated. Please provide the tool name as TOOL/SUBTOOL or TOOL." ) modules_repo = ModulesRepo() modules_repo.get_modules_file_tree() diff --git a/tests/modules/module_test.py b/tests/modules/module_test.py index fbad1c9fc5..a4559ffde5 100644 --- a/tests/modules/module_test.py +++ b/tests/modules/module_test.py @@ -14,3 +14,14 @@ def test_modules_test_check_inputs(self): meta_builder._check_inputs() os.chdir(cwd) assert "Cannot find directory" in str(excinfo.value) + + +def test_modules_test_no_name_no_prompts(self): + """Test the check_inputs() function - raise UserWarning prompts are deactivated and module name is not provided.""" + cwd = os.getcwd() + os.chdir(self.nfcore_modules) + meta_builder = nf_core.modules.ModulesTest(None, True, "") + with pytest.raises(UserWarning) as excinfo: + meta_builder._check_inputs() + os.chdir(cwd) + assert "Tool name not provided and prompts deactivated." in str(excinfo.value) diff --git a/tests/test_modules.py b/tests/test_modules.py index 00cc89c36d..b12333b51d 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -116,4 +116,5 @@ def test_modulesrepo_class(self): from .modules.module_test import ( test_modules_test_check_inputs, + test_modules_test_no_name_no_prompts, ) From d59497b9fde438d635ccbd0d9cc860356eaf5665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 27 Apr 2022 16:53:46 +0200 Subject: [PATCH 42/93] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b4c6c0c97..5e72c06b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Escaped test run output before logging it, to avoid a rich ` MarkupError` - Add a new command `nf-core modules mulled` which can generate the name for a multi-tool container image. +- Add a new command `nf-core modules test` which runs pytests locally. ## [v2.3.2 - Mercury Vulture Fixed Formatting](https://github.com/nf-core/tools/releases/tag/2.3.2) - [2022-03-24] From 5a61003ffb42956b01489dad3dab2b1b524544ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Thu, 28 Apr 2022 09:57:41 +0200 Subject: [PATCH 43/93] specify test files as test_*.py in pytest.ini --- pytest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/pytest.ini b/pytest.ini index 57209fee41..652bdf8e53 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,3 +3,4 @@ filterwarnings = ignore::pytest.PytestRemovedIn8Warning:_pytest.nodes:140 testpaths = tests +python_files = test_*.py From 98f02390ce657338c5cfcc099f0179612e9fa407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Thu, 28 Apr 2022 14:31:30 +0200 Subject: [PATCH 44/93] apply comments from PR review --- nf_core/modules/module_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 25e4991a4b..57ca2c6932 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -9,6 +9,7 @@ import pytest import sys import rich +from pathlib import Path import nf_core.utils from .modules_repo import ModulesRepo @@ -52,7 +53,6 @@ def __init__( self.module_name = module_name self.no_prompts = no_prompts self.pytest_args = pytest_args - self.module_dir = None def run(self): """Run test steps""" @@ -80,11 +80,11 @@ def _check_inputs(self): choices=modules_repo.modules_avail_module_names, style=nf_core.utils.nfcore_question_style, ).ask() - self.module_dir = os.path.join("modules", *self.module_name.split("/")) + module_dir = Path("modules") / self.module_name # First, sanity check that the module directory exists - if not os.path.isdir(self.module_dir): - raise UserWarning(f"Cannot find directory '{self.module_dir}'. Should be TOOL/SUBTOOL or TOOL") + if not module_dir.is_dir(): + raise UserWarning(f"Cannot find directory '{module_dir}'. Should be TOOL/SUBTOOL or TOOL") def _set_profile(self): """Set $PROFILE env variable. @@ -117,7 +117,7 @@ def _run_pytests(self): console.print("[black]" + "─" * console.width) # Set pytest arguments - command_args = ["--tag", f"{self.module_name}", "--symlink", "--keep-workflow-wd"] + command_args = ["--tag", f"{self.module_name}", "--symlink", "--keep-workflow-wd", "--git-aware"] command_args += self.pytest_args # Run pytest From 86dabbfd7fdcc7030a6bcffd3284546292617ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 29 Apr 2022 14:12:52 +0200 Subject: [PATCH 45/93] check if profile is available --- nf_core/modules/module_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 57ca2c6932..8ca1027419 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -9,6 +9,8 @@ import pytest import sys import rich +import subprocess +import shlex from pathlib import Path import nf_core.utils @@ -62,6 +64,7 @@ def run(self): ) self._check_inputs() self._set_profile() + self._check_profile() self._run_pytests() def _check_inputs(self): @@ -110,6 +113,14 @@ def _set_profile(self): os.environ["PROFILE"] = profile log.info(f"Setting environment variable '$PROFILE' to '{profile}'") + def _check_profile(self): + """Check if profile is available""" + profile = os.environ.get("PROFILE") + try: + profile_check = subprocess.check_output(shlex.split(f"{profile} --help"), stderr=subprocess.STDOUT, shell=True) + except subprocess.CalledProcessError as e: + raise UserWarning(f"Error with profile {profile} (exit code {e.returncode})\n[red]{e.output.decode()}") + def _run_pytests(self): """Given a module name, run tests.""" # Print nice divider line From 37750dc5dd9605e3180941bdfafcc229bad02f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 29 Apr 2022 14:41:02 +0200 Subject: [PATCH 46/93] modify linting --- nf_core/modules/module_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 8ca1027419..c2510d3ee9 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -117,7 +117,9 @@ def _check_profile(self): """Check if profile is available""" profile = os.environ.get("PROFILE") try: - profile_check = subprocess.check_output(shlex.split(f"{profile} --help"), stderr=subprocess.STDOUT, shell=True) + profile_check = subprocess.check_output( + shlex.split(f"{profile} --help"), stderr=subprocess.STDOUT, shell=True + ) except subprocess.CalledProcessError as e: raise UserWarning(f"Error with profile {profile} (exit code {e.returncode})\n[red]{e.output.decode()}") From a0ac5a1de486bf39a415a7c23eaa72fef65f3bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 12:42:59 +0200 Subject: [PATCH 47/93] Add nf-core/modules folder warning Co-authored-by: Gisela Gabernet --- nf_core/modules/module_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index c2510d3ee9..6507c58219 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -87,7 +87,7 @@ def _check_inputs(self): # First, sanity check that the module directory exists if not module_dir.is_dir(): - raise UserWarning(f"Cannot find directory '{module_dir}'. Should be TOOL/SUBTOOL or TOOL") + raise UserWarning(f"Cannot find directory '{module_dir}'. Should be TOOL/SUBTOOL or TOOL. Are you running the tests inside the nf-core/modules main directory?") def _set_profile(self): """Set $PROFILE env variable. From e45dbfd3e39f3e97cfc7304a7df313481b4c0e8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 12:44:22 +0200 Subject: [PATCH 48/93] Add valid profile check Co-authored-by: Fabian Egli --- nf_core/modules/module_test.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 6507c58219..65006a6ced 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -117,9 +117,16 @@ def _check_profile(self): """Check if profile is available""" profile = os.environ.get("PROFILE") try: - profile_check = subprocess.check_output( - shlex.split(f"{profile} --help"), stderr=subprocess.STDOUT, shell=True - ) + + # Make sure the profile read from the environment is a valid Nextflow profile. + valid_nextflow_profiles = ["docker", "singularity", "podman", "shifter", "charliecloud", "conda"] + if profile in valid_nextflow_profiles: + profile_check = subprocess.check_output([profile, "--help"], stderr=subprocess.STDOUT) + else: + raise UserWarning( + f"The profile '{profile}' set in the shell environment is not a valid.\n" + f"Valid Nextflow profiles are {valid_nextflow_profiles}." + ) except subprocess.CalledProcessError as e: raise UserWarning(f"Error with profile {profile} (exit code {e.returncode})\n[red]{e.output.decode()}") From cfa2e1cc83dc34ad18e9fd1b1b01e8b8f7be745d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 13:05:39 +0200 Subject: [PATCH 49/93] fix linting --- nf_core/modules/module_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 65006a6ced..781fb3b2f1 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -87,7 +87,9 @@ def _check_inputs(self): # First, sanity check that the module directory exists if not module_dir.is_dir(): - raise UserWarning(f"Cannot find directory '{module_dir}'. Should be TOOL/SUBTOOL or TOOL. Are you running the tests inside the nf-core/modules main directory?") + raise UserWarning( + f"Cannot find directory '{module_dir}'. Should be TOOL/SUBTOOL or TOOL. Are you running the tests inside the nf-core/modules main directory?" + ) def _set_profile(self): """Set $PROFILE env variable. @@ -117,7 +119,6 @@ def _check_profile(self): """Check if profile is available""" profile = os.environ.get("PROFILE") try: - # Make sure the profile read from the environment is a valid Nextflow profile. valid_nextflow_profiles = ["docker", "singularity", "podman", "shifter", "charliecloud", "conda"] if profile in valid_nextflow_profiles: From 5dce2624a0be9ac0baa80ce0e04e3db92223d6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 13:14:23 +0200 Subject: [PATCH 50/93] avoid using shell=True when checking profile --- nf_core/modules/module_test.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 781fb3b2f1..75c7cbb333 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -12,6 +12,7 @@ import subprocess import shlex from pathlib import Path +from shutil import which import nf_core.utils from .modules_repo import ModulesRepo @@ -118,18 +119,16 @@ def _set_profile(self): def _check_profile(self): """Check if profile is available""" profile = os.environ.get("PROFILE") - try: - # Make sure the profile read from the environment is a valid Nextflow profile. - valid_nextflow_profiles = ["docker", "singularity", "podman", "shifter", "charliecloud", "conda"] - if profile in valid_nextflow_profiles: - profile_check = subprocess.check_output([profile, "--help"], stderr=subprocess.STDOUT) - else: - raise UserWarning( - f"The profile '{profile}' set in the shell environment is not a valid.\n" - f"Valid Nextflow profiles are {valid_nextflow_profiles}." - ) - except subprocess.CalledProcessError as e: - raise UserWarning(f"Error with profile {profile} (exit code {e.returncode})\n[red]{e.output.decode()}") + # Make sure the profile read from the environment is a valid Nextflow profile. + valid_nextflow_profiles = ["docker", "singularity", "podman", "shifter", "charliecloud", "conda"] + if profile in valid_nextflow_profiles: + if not which(profile): + raise UserWarning(f"The PROFILE '{profile}' set in the shell environment is not available.") + else: + raise UserWarning( + f"The PROFILE '{profile}' set in the shell environment is not valid.\n" + f"Valid Nextflow profiles are {valid_nextflow_profiles}." + ) def _run_pytests(self): """Given a module name, run tests.""" From 930601d3cf03302c40b36d9e271ab9ec6d92f146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 13:54:45 +0200 Subject: [PATCH 51/93] Apply suggestions from code review Co-authored-by: Phil Ewels --- nf_core/modules/module_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 75c7cbb333..8ca12df6d5 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -120,21 +120,21 @@ def _check_profile(self): """Check if profile is available""" profile = os.environ.get("PROFILE") # Make sure the profile read from the environment is a valid Nextflow profile. - valid_nextflow_profiles = ["docker", "singularity", "podman", "shifter", "charliecloud", "conda"] + valid_nextflow_profiles = ["docker", "singularity", "conda"] if profile in valid_nextflow_profiles: if not which(profile): - raise UserWarning(f"The PROFILE '{profile}' set in the shell environment is not available.") + raise UserWarning(f"Command '{profile}' not found - is it installed?") else: raise UserWarning( f"The PROFILE '{profile}' set in the shell environment is not valid.\n" - f"Valid Nextflow profiles are {valid_nextflow_profiles}." + f"Valid Nextflow profiles are '{', '.join(valid_nextflow_profiles)}'." ) def _run_pytests(self): """Given a module name, run tests.""" # Print nice divider line console = rich.console.Console() - console.print("[black]" + "─" * console.width) + console.rule(self.module_name, style="black") # Set pytest arguments command_args = ["--tag", f"{self.module_name}", "--symlink", "--keep-workflow-wd", "--git-aware"] From 0d4f8133aaa54db9963d158bb61eca0bcbfe2253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 2 May 2022 13:59:54 +0200 Subject: [PATCH 52/93] remove unnecessary libraryes --- nf_core/modules/module_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nf_core/modules/module_test.py b/nf_core/modules/module_test.py index 8ca12df6d5..35658dd26b 100644 --- a/nf_core/modules/module_test.py +++ b/nf_core/modules/module_test.py @@ -9,8 +9,6 @@ import pytest import sys import rich -import subprocess -import shlex from pathlib import Path from shutil import which From b4343ae5945cd435e0c1d926f712d9f19f749563 Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Wed, 4 May 2022 15:40:42 +0200 Subject: [PATCH 53/93] replace a weird with a normal dash --- nf_core/launch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/launch.py b/nf_core/launch.py index ede47d0f22..b0c3e565f7 100644 --- a/nf_core/launch.py +++ b/nf_core/launch.py @@ -420,7 +420,7 @@ def prompt_param(self, param_id, param_obj, is_required, answers): # If required and got an empty reponse, ask again while type(answer[param_id]) is str and answer[param_id].strip() == "" and is_required: - log.error("'–-{}' is required".format(param_id)) + log.error("'--{}' is required".format(param_id)) answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style) # Ignore if empty From c9acfb5de8b240bb43018f432880d4830bb6a950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 6 May 2022 11:05:33 +0200 Subject: [PATCH 54/93] add documentation for modules mulled command --- README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/README.md b/README.md index be0054533c..e9d777f0b5 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ A python package with helper tools for the nf-core community. - [`modules create-test-yml` - Create the `test.yml` file for a module](#create-a-module-test-config-file) - [`modules lint` - Check a module against nf-core guidelines](#check-a-module-against-nf-core-guidelines) - [`modules bump-versions` - Bump software versions of modules](#bump-bioconda-and-container-versions-of-modules-in) + - [`modules mulled` - Generate the name for a multi-tool container image](#generate the name for a multi-tool container image) - [Citation](#citation) @@ -1339,6 +1340,25 @@ bump-versions: star/align: "2.6.1d" ``` +### Generate the name for a multi-tool container image + +When you want to use an image of a multi-tool container, and you know the specific dependencies and their versions of that container, you can use the `nf-core modules mulled` helper tool. This tool generates the name of a BioContainers mulled image. + +```console +$ nf-core modules mulled pysam==0.16.0.1 biopython==1.78 + + ,--./,-. + ___ __ __ __ ___ /,-._.--~\ + |\ | |__ __ / ` / \ |__) |__ } { + | \| | \__, \__/ | \ |___ \`-._,-`-, + `._,._,' + + nf-core/tools version 2.4 + + +mulled-v2-3a59640f3fe1ed11819984087d31d68600200c3f:185a25ca79923df85b58f42deb48f5ac4481e91f-0 +``` + ## Citation If you use `nf-core tools` in your work, please cite the `nf-core` publication as follows: From 9fd03522fc83aa338fa05b5a531dd78366fe8b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 6 May 2022 12:44:06 +0200 Subject: [PATCH 55/93] add documentation for modules test command --- README.md | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e9d777f0b5..04ed950359 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,9 @@ A python package with helper tools for the nf-core community. - [`modules create` - Create a module from the template](#create-a-new-module) - [`modules create-test-yml` - Create the `test.yml` file for a module](#create-a-module-test-config-file) - [`modules lint` - Check a module against nf-core guidelines](#check-a-module-against-nf-core-guidelines) + - [`modules test` - Run the tests for a module](#run-the-tests-for-a-module-using-pytest) - [`modules bump-versions` - Bump software versions of modules](#bump-bioconda-and-container-versions-of-modules-in) - - [`modules mulled` - Generate the name for a multi-tool container image](#generate the name for a multi-tool container image) + - [`modules mulled` - Generate the name for a multi-tool container image](#generate-the-name-for-a-multi-tool-container-image) - [Citation](#citation) @@ -1297,6 +1298,57 @@ INFO Linting module: star/align ╰──────────────────────╯ ``` +### Run the tests for a module using pytest + +To run unit tests of a module that you have installed or the test created by the command [`nf-core mdoules create-test-yml`](#create-a-module-test-config-file), you can use `nf-core modules test` command. This command runs the tests specified in `modules/tests/software///test.yml` file using [pytest](https://pytest-workflow.readthedocs.io/en/stable/). + +You can specify the module name in the form TOOL/SUBTOOL in command line or provide it later by prompts. + +```console +$ nf-core modules test fastqc + ,--./,-. + ___ __ __ __ ___ /,-._.--~\ + |\ | |__ __ / ` / \ |__) |__ } { + | \| | \__, \__/ | \ |___ \`-._,-`-, + `._,._,' + + nf-core/tools version 2.4 + +? Choose software profile Docker +INFO Setting environment variable '$PROFILE' to 'docker' +INFO Running pytest for module 'fastqc' + +=============================================================== test session starts ================================================================ +platform darwin -- Python 3.9.12, pytest-7.1.2, pluggy-1.0.0 +rootdir: ~/modules, configfile: pytest.ini +plugins: workflow-1.6.0 +collecting ... +collected 761 items + +fastqc single-end: + command: nextflow run ./tests/modules/fastqc/ -entry test_fastqc_single_end -c ./tests/config/nextflow.config -c ./tests/modules/fastqc/nextflow.config -c ./tests/modules/fastqc/nextflow.config + directory: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_single-end + stdout: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_single-end/log.out + stderr: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_single-end/log.err +'fastqc single-end' done. + +fastqc paired-end: + command: nextflow run ./tests/modules/fastqc/ -entry test_fastqc_paired_end -c ./tests/config/nextflow.config -c ./tests/modules/fastqc/nextflow.config -c ./tests/modules/fastqc/nextflow.config + directory: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_paired-end + stdout: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_paired-end/log.out + stderr: /var/folders/lt/b3cs9y610fg_13q14dckwcvm0000gn/T/pytest_workflow_ahvulf1v/fastqc_paired-end/log.err +'fastqc paired-end' done. + +tests/test_versions_yml.py sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 17%] +ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 38%] +ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 59%] +sssssssssssssssssssssssssssssssssssssssssssssssssssssssssss..ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss ssss [ 80%] +ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 98%] +tests/modules/fastqc/test.yml ........ +Keeping temporary directories and logs. Use '--kwd' or '--keep-workflow-wd' to disable this behaviour. +=================================================== 10 passed, 751 skipped, 479 warnings in 50.76s =================================================== +``` + ### Bump bioconda and container versions of modules in If you are contributing to the `nf-core/modules` repository and want to bump bioconda and container versions of certain modules, you can use the `nf-core modules bump-versions` helper tool. This will bump the bioconda version of a single or all modules to the latest version and also fetch the correct Docker and Singularity container tags. From 1afd36b3f168872a58024c287a5ec57e281d1466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 6 May 2022 13:48:55 +0200 Subject: [PATCH 56/93] Add biocontainers reference Co-authored-by: Moritz E. Beber --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 04ed950359..949a11b424 100644 --- a/README.md +++ b/README.md @@ -1394,7 +1394,7 @@ bump-versions: ### Generate the name for a multi-tool container image -When you want to use an image of a multi-tool container, and you know the specific dependencies and their versions of that container, you can use the `nf-core modules mulled` helper tool. This tool generates the name of a BioContainers mulled image. +When you want to use an image of a multi-tool container and you know the specific dependencies and their versions of that container, for example, by looking them up in the [BioContainers hash.tsv](https://github.com/BioContainers/multi-package-containers/blob/master/combinations/hash.tsv), you can use the `nf-core modules mulled` helper tool. This tool generates the name of a BioContainers mulled image. ```console $ nf-core modules mulled pysam==0.16.0.1 biopython==1.78 From 90ec2210369d966fc4d3575ff571f4edeab6df4a Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 29 Mar 2022 17:07:27 +0200 Subject: [PATCH 57/93] First stab at writing a central utils function for GitHub API calls. Uses authentication from local gh CLI or a supplied bearer token if found. Done to address nf-core/tools#1496 Bit scared it'll break something, so ideally needs extensive testing. --- nf_core/modules/module_utils.py | 10 +-- nf_core/modules/modules_repo.py | 10 +-- nf_core/sync.py | 88 +++++++------------------ nf_core/utils.py | 111 ++++++++++++++++++++++++++------ 4 files changed, 125 insertions(+), 94 deletions(-) diff --git a/nf_core/modules/module_utils.py b/nf_core/modules/module_utils.py index c2fc542759..986d296bbe 100644 --- a/nf_core/modules/module_utils.py +++ b/nf_core/modules/module_utils.py @@ -35,7 +35,7 @@ def module_exist_in_repo(module_name, modules_repo): api_url = ( f"https://api.github.com/repos/{modules_repo.name}/contents/modules/{module_name}?ref={modules_repo.branch}" ) - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) return not (response.status_code == 404) @@ -65,7 +65,7 @@ def get_module_git_log(module_name, modules_repo=None, per_page=30, page_nbr=1, api_url += f"&since={since}" log.debug(f"Fetching commit history of module '{module_name}' from github API") - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 200: commits = response.json() @@ -101,7 +101,7 @@ def get_commit_info(commit_sha, repo_name="nf-core/modules"): ) api_url = f"https://api.github.com/repos/{repo_name}/commits/{commit_sha}?stats=false" log.debug(f"Fetching commit metadata for commit at {commit_sha}") - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 200: commit = response.json() message = commit["commit"]["message"].partition("\n")[0] @@ -266,7 +266,7 @@ def local_module_equal_to_commit(local_files, module_name, modules_repo, commit_ for i, file in enumerate(files_to_check): # Download remote copy and compare api_url = f"{module_base_url}/{file}" - r = requests.get(url=api_url, auth=nf_core.utils.github_api_auto_auth()) + r = nf_core.utils.call_github_api(api_url, raise_error=False) if r.status_code != 200: log.debug(f"Could not download remote copy of file module {module_name}/{file}") log.debug(api_url) @@ -414,7 +414,7 @@ def verify_pipeline_dir(dir): modules_is_software = False for repo_name in repo_names: api_url = f"https://api.github.com/repos/{repo_name}/contents" - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 404: missing_remote.append(repo_name) if repo_name == "nf-core/software": diff --git a/nf_core/modules/modules_repo.py b/nf_core/modules/modules_repo.py index a625a0db06..0fc7189d64 100644 --- a/nf_core/modules/modules_repo.py +++ b/nf_core/modules/modules_repo.py @@ -43,7 +43,7 @@ def __init__(self, repo="nf-core/modules", branch=None): def get_default_branch(self): """Get the default branch for a GitHub repo""" api_url = f"https://api.github.com/repos/{self.name}" - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 200: self.branch = response.json()["default_branch"] log.debug(f"Found default branch to be '{self.branch}'") @@ -58,7 +58,7 @@ def verify_modules_repo(self): # Check if repository exist api_url = f"https://api.github.com/repos/{self.name}/branches" - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 200: branches = [branch["name"] for branch in response.json()] if self.branch not in branches: @@ -67,7 +67,7 @@ def verify_modules_repo(self): raise LookupError(f"Repository '{self.name}' is not available on GitHub") api_url = f"https://api.github.com/repos/{self.name}/contents?ref={self.branch}" - response = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + response = nf_core.utils.call_github_api(api_url, raise_error=False) if response.status_code == 200: dir_names = [entry["name"] for entry in response.json() if entry["type"] == "dir"] if "modules" not in dir_names: @@ -86,7 +86,7 @@ def get_modules_file_tree(self): self.modules_avail_module_names """ api_url = "https://api.github.com/repos/{}/git/trees/{}?recursive=1".format(self.name, self.branch) - r = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + r = nf_core.utils.call_github_api(api_url, raise_error=False) if r.status_code == 404: raise LookupError("Repository / branch not found: {} ({})\n{}".format(self.name, self.branch, api_url)) elif r.status_code != 200: @@ -157,7 +157,7 @@ def download_gh_file(self, dl_filename, api_url): os.makedirs(dl_directory) # Call the GitHub API - r = requests.get(api_url, auth=nf_core.utils.github_api_auto_auth()) + r = nf_core.utils.call_github_api(api_url, raise_error=False) if r.status_code != 200: raise LookupError("Could not fetch {} file: {}\n {}".format(self.name, r.status_code, api_url)) result = r.json() diff --git a/nf_core/sync.py b/nf_core/sync.py index fb1d9f4db0..389da1fc32 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -316,75 +316,31 @@ def make_pull_request(self): ).format(tag=nf_core.__version__) # Make new pull-request - pr_content = { - "title": pr_title, - "body": pr_body_text, - "maintainer_can_modify": True, - "head": self.merge_branch, - "base": self.from_branch, - } - stderr = rich.console.Console(stderr=True, force_terminal=nf_core.utils.rich_force_colors()) - - while True: + log.debug("Submitting PR to GitHub API") + with requests_cache.disabled(): try: - log.debug("Submitting PR to GitHub API") - returned_data_prettyprint = "" - r_headers_pp = "" - with requests_cache.disabled(): - r = requests.post( - url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) - try: - self.gh_pr_returned_data = json.loads(r.content) - returned_data_prettyprint = json.dumps(dict(self.gh_pr_returned_data), indent=4) - r_headers_pp = json.dumps(dict(r.headers), indent=4) - except: - self.gh_pr_returned_data = r.content - returned_data_prettyprint = r.content - r_headers_pp = r.headers - log.error("Could not parse JSON response from GitHub API!") - stderr.print_exception() - - # Dump the responses to the log just in case.. - log.debug(f"PR response from GitHub. Data:\n{returned_data_prettyprint}\n\nHeaders:\n{r_headers_pp}") - - # PR worked - if r.status_code == 201: - self.pr_url = self.gh_pr_returned_data["html_url"] - log.debug(f"GitHub API PR worked, return code 201") - log.info(f"GitHub PR created: {self.gh_pr_returned_data['html_url']}") - break - - # Returned 403 error - too many simultaneous requests - # https://github.com/nf-core/tools/issues/911 - if r.status_code == 403: - log.debug(f"GitHub API PR failed with 403 error") - wait_time = float(re.sub("[^0-9]", "", str(r.headers.get("Retry-After", 0)))) - if wait_time == 0: - log.debug("Couldn't find 'Retry-After' header, guessing a length of time to wait") - wait_time = random.randrange(10, 60) - log.warning( - f"Got 403 code - probably the abuse protection. Trying again after {wait_time} seconds.." - ) - time.sleep(wait_time) - - # Something went wrong - else: - raise PullRequestException( - f"GitHub API returned code {r.status_code}: \n\n{returned_data_prettyprint}\n\n{r_headers_pp}" - ) - # Don't catch the PullRequestException that we raised inside - except PullRequestException: - raise - # Do catch any other exceptions that we hit - except Exception as e: - stderr.print_exception() - raise PullRequestException( - f"Something went badly wrong - {e}: \n\n{returned_data_prettyprint}\n\n{r_headers_pp}" + r = nf_core.utils.call_github_api( + f"https://api.github.com/repos/{self.gh_repo}/pulls", + post_data={ + "title": pr_title, + "body": pr_body_text, + "maintainer_can_modify": True, + "head": self.merge_branch, + "base": self.from_branch, + }, + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + return_ok=[201], + return_retry=[403], ) + except Exception as e: + stderr.print_exception(e) + raise PullRequestException(f"Something went badly wrong - {e}") + else: + self.gh_pr_returned_data = r.json() + self.pr_url = self.gh_pr_returned_data["html_url"] + log.debug(f"GitHub API PR worked, return code 201") + log.info(f"GitHub PR created: {self.gh_pr_returned_data['html_url']}") def close_open_template_merge_prs(self): """Get all template merging branches (starting with 'nf-core-template-merge-') diff --git a/nf_core/utils.py b/nf_core/utils.py index 34660d431c..be59db353a 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -15,6 +15,7 @@ import os import prompt_toolkit import questionary +import random import re import requests import requests_cache @@ -80,17 +81,6 @@ def rich_force_colors(): return None -def github_api_auto_auth(): - try: - with open(os.path.join(os.path.expanduser("~/.config/gh/hosts.yml")), "r") as fh: - auth = yaml.safe_load(fh) - log.debug("Auto-authenticating GitHub API as '@{}'".format(auth["github.com"]["user"])) - return requests.auth.HTTPBasicAuth(auth["github.com"]["user"], auth["github.com"]["oauth_token"]) - except Exception as e: - log.debug(f"Couldn't auto-auth for GitHub: [red]{e}") - return None - - class Pipeline(object): """Object to hold information about a local pipeline. @@ -386,6 +376,91 @@ def poll_nfcore_web_api(api_url, post_data=None): return web_response +def call_github_api(url, post_data=None, return_ok=[200, 201], return_retry=[], auth=None, raise_error=True): + """ + Send a request to the GitHub API. + Uses authentication (to avoid rate limiting) if available. + + Args: + url (str): The full URL of the GitHub API request. + + Returns: + data (dict): The JSON response from the GitHub API. + """ + # Class for Bearer token authentication + # https://stackoverflow.com/a/58055668/713980 + class BearerAuth(requests.auth.AuthBase): + def __init__(self, token): + self.token = token + + def __call__(self, r): + r.headers["authorization"] = f"Bearer {self.token}" + return r + + # Default auth if we're running and the gh CLI tool is installed + if auth is None: + try: + with open(os.path.join(os.path.expanduser("~/.config/gh/hosts.yml")), "r") as fh: + gh_cli_config = yaml.safe_load(fh) + log.debug("Auto-authenticating GitHub API as '@{}'".format(gh_cli_config["github.com"]["user"])) + auth = requests.auth.HTTPBasicAuth( + gh_cli_config["github.com"]["user"], gh_cli_config["github.com"]["oauth_token"] + ) + except Exception as e: + log.debug(f"Couldn't auto-auth for GitHub: [red]{e}") + + # Default auth if we have a GitHub Token (eg. GitHub Actions CI) + if os.environ.get("GITHUB_TOKEN") is not None and auth is None: + auth = BearerAuth(os.environ["GITHUB_TOKEN"]) + + # Start the loop for a retry mechanism + while True: + # GET request + if post_data is None: + r = requests.get(url, auth=auth) + # POST request + else: + r = requests.post(url, json=post_data, auth=auth) + + try: + r_headers_pp = json.dumps(dict(r.headers), indent=4) + r_data_pp = json.dumps(dict(r.json()), indent=4) + except Exception as e: + log.debug(r.headers) + log.debug(r.content) + if raise_error: + raise AssertionError(f"Could not parse JSON response from GitHub API! {e}") + else: + return r + + # Failed but expected - try again + # Added due to 403 error: too many simultaneous requests + # See https://github.com/nf-core/tools/issues/911 + if r.status_code in return_retry: + log.debug(f"GitHub API PR failed - got return code {r.status_code}") + log.debug(r_headers_pp) + log.debug(r_data_pp) + wait_time = float(re.sub("[^0-9]", "", str(r.headers.get("Retry-After", 0)))) + if wait_time == 0: + log.debug("Couldn't find 'Retry-After' header, guessing a length of time to wait") + wait_time = random.randrange(10, 60) + log.warning(f"Got API return code {r.status_code}. Trying again after {wait_time} seconds..") + time.sleep(wait_time) + + # Unexpected error - raise + elif r.status_code not in return_ok: + log.debug(r_headers_pp) + log.debug(r_data_pp) + if raise_error: + raise AssertionError(f"GitHub API PR failed - got return code {r.status_code} from {url}") + else: + return r + + # Success! + else: + return r + + def anaconda_package(dep, dep_channels=["conda-forge", "bioconda", "defaults"]): """Query conda package information. @@ -627,9 +702,9 @@ def prompt_remote_pipeline_name(wfs): else: if pipeline.count("/") == 1: try: - gh_response = requests.get(f"https://api.github.com/repos/{pipeline}") - assert gh_response.json().get("message") != "Not Found" - except AssertionError: + nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}") + except Exception: + # No repo found - pass and raise error at the end pass else: return pipeline @@ -706,7 +781,7 @@ def get_repo_releases_branches(pipeline, wfs): ) # Get releases from GitHub API - rel_r = requests.get(f"https://api.github.com/repos/{pipeline}/releases") + rel_r = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/releases") # Check that this repo existed try: @@ -714,13 +789,13 @@ def get_repo_releases_branches(pipeline, wfs): except AssertionError: raise AssertionError(f"Not able to find pipeline '{pipeline}'") except AttributeError: - # When things are working we get a list, which doesn't work with .get() + # Success! We have a list, which doesn't work with .get() which is looking for a dict key wf_releases = list(sorted(rel_r.json(), key=lambda k: k.get("published_at_timestamp", 0), reverse=True)) # Get release tag commit hashes if len(wf_releases) > 0: # Get commit hash information for each release - tags_r = requests.get(f"https://api.github.com/repos/{pipeline}/tags") + tags_r = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/tags") for tag in tags_r.json(): for release in wf_releases: if tag["name"] == release["tag_name"]: @@ -731,7 +806,7 @@ def get_repo_releases_branches(pipeline, wfs): raise AssertionError(f"Not able to find pipeline '{pipeline}'") # Get branch information from github api - should be no need to check if the repo exists again - branch_response = requests.get(f"https://api.github.com/repos/{pipeline}/branches") + branch_response = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/branches") for branch in branch_response.json(): if ( branch["name"] != "TEMPLATE" From e0f984f4ed4be0033430d669d9c3ec676c627a7a Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 30 Mar 2022 14:26:55 +0200 Subject: [PATCH 58/93] Remove superfluous dict() call --- nf_core/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/utils.py b/nf_core/utils.py index be59db353a..a8be9c85a6 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -424,7 +424,7 @@ def __call__(self, r): try: r_headers_pp = json.dumps(dict(r.headers), indent=4) - r_data_pp = json.dumps(dict(r.json()), indent=4) + r_data_pp = json.dumps(r.json(), indent=4) except Exception as e: log.debug(r.headers) log.debug(r.content) From 464510e70de576ea5c6e2b6db436b48de8df34b0 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 30 Mar 2022 22:21:23 +0200 Subject: [PATCH 59/93] Fix sync pytests --- nf_core/sync.py | 2 +- nf_core/utils.py | 4 ++-- tests/test_sync.py | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 389da1fc32..d4452ecd2c 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -334,7 +334,7 @@ def make_pull_request(self): return_retry=[403], ) except Exception as e: - stderr.print_exception(e) + stderr.print_exception() raise PullRequestException(f"Something went badly wrong - {e}") else: self.gh_pr_returned_data = r.json() diff --git a/nf_core/utils.py b/nf_core/utils.py index a8be9c85a6..ead6a7fd35 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -417,10 +417,10 @@ def __call__(self, r): while True: # GET request if post_data is None: - r = requests.get(url, auth=auth) + r = requests.get(url=url, auth=auth) # POST request else: - r = requests.post(url, json=post_data, auth=auth) + r = requests.post(url=url, json=post_data, auth=auth) try: r_headers_pp = json.dumps(dict(r.headers), indent=4) diff --git a/tests/test_sync.py b/tests/test_sync.py index 68c96e2929..a0d009638f 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -200,6 +200,9 @@ def __init__(self, data, status_code): self.content = json.dumps(data) self.headers = {"content-encoding": "test", "connection": "fake"} + def json(self): + return json.loads(self.content) + if kwargs["url"] == "https://api.github.com/repos/no_existing_pr/response/pulls": response_data = {"html_url": "great_success"} return MockResponse(response_data, 201) @@ -229,4 +232,4 @@ def test_make_pull_request_bad_response(self, mock_get, mock_post): psync.make_pull_request() raise UserWarning("Should have hit an exception") except nf_core.sync.PullRequestException as e: - assert e.args[0].startswith("GitHub API returned code 404:") + assert e.args[0].startswith("Something went badly wrong - GitHub API PR failed - got return code 404") From fb2c4cf531a2386bd0084d9b6c44edaef4f83b75 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 30 Mar 2022 22:41:05 +0200 Subject: [PATCH 60/93] Better debug logging. Also removed function prefix from calls that were in the same file --- nf_core/utils.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/nf_core/utils.py b/nf_core/utils.py index ead6a7fd35..a1dba30c6a 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -387,6 +387,11 @@ def call_github_api(url, post_data=None, return_ok=[200, 201], return_retry=[], Returns: data (dict): The JSON response from the GitHub API. """ + + auth_mode = None + if auth is not None: + auth_mode = "supplied to function" + # Class for Bearer token authentication # https://stackoverflow.com/a/58055668/713980 class BearerAuth(requests.auth.AuthBase): @@ -398,21 +403,25 @@ def __call__(self, r): return r # Default auth if we're running and the gh CLI tool is installed - if auth is None: + gh_cli_config_fn = os.path.expanduser("~/.config/gh/hosts.yml") + if auth is None and os.path.exists(gh_cli_config_fn): try: - with open(os.path.join(os.path.expanduser("~/.config/gh/hosts.yml")), "r") as fh: + with open(gh_cli_config_fn, "r") as fh: gh_cli_config = yaml.safe_load(fh) - log.debug("Auto-authenticating GitHub API as '@{}'".format(gh_cli_config["github.com"]["user"])) auth = requests.auth.HTTPBasicAuth( gh_cli_config["github.com"]["user"], gh_cli_config["github.com"]["oauth_token"] ) + auth_mode = f"gh CLI config: {gh_cli_config['github.com']['user']}" except Exception as e: - log.debug(f"Couldn't auto-auth for GitHub: [red]{e}") + log.debug(f"Couldn't auto-auth with GitHub CLI auth: [red]{e}") # Default auth if we have a GitHub Token (eg. GitHub Actions CI) if os.environ.get("GITHUB_TOKEN") is not None and auth is None: + auth_mode = "GITHUB_TOKEN" auth = BearerAuth(os.environ["GITHUB_TOKEN"]) + log.debug(f"Fetching GitHub API: {url} (auth: {auth_mode})") + # Start the loop for a retry mechanism while True: # GET request @@ -702,7 +711,7 @@ def prompt_remote_pipeline_name(wfs): else: if pipeline.count("/") == 1: try: - nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}") + call_github_api(f"https://api.github.com/repos/{pipeline}") except Exception: # No repo found - pass and raise error at the end pass @@ -781,7 +790,7 @@ def get_repo_releases_branches(pipeline, wfs): ) # Get releases from GitHub API - rel_r = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/releases") + rel_r = call_github_api(f"https://api.github.com/repos/{pipeline}/releases") # Check that this repo existed try: @@ -795,7 +804,7 @@ def get_repo_releases_branches(pipeline, wfs): # Get release tag commit hashes if len(wf_releases) > 0: # Get commit hash information for each release - tags_r = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/tags") + tags_r = call_github_api(f"https://api.github.com/repos/{pipeline}/tags") for tag in tags_r.json(): for release in wf_releases: if tag["name"] == release["tag_name"]: @@ -806,7 +815,7 @@ def get_repo_releases_branches(pipeline, wfs): raise AssertionError(f"Not able to find pipeline '{pipeline}'") # Get branch information from github api - should be no need to check if the repo exists again - branch_response = nf_core.utils.call_github_api(f"https://api.github.com/repos/{pipeline}/branches") + branch_response = call_github_api(f"https://api.github.com/repos/{pipeline}/branches") for branch in branch_response.json(): if ( branch["name"] != "TEMPLATE" From d37dbff0867844a7acd460d63ffe8daa76672c85 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 13 Apr 2022 16:27:09 +0200 Subject: [PATCH 61/93] Try to rewrite GitHub API calls to use a single shared Session --- nf_core/modules/module_utils.py | 19 ++- nf_core/modules/modules_repo.py | 14 +- nf_core/sync.py | 13 +- nf_core/utils.py | 221 ++++++++++++++++++++------------ 4 files changed, 161 insertions(+), 106 deletions(-) diff --git a/nf_core/modules/module_utils.py b/nf_core/modules/module_utils.py index 986d296bbe..23c5ec526f 100644 --- a/nf_core/modules/module_utils.py +++ b/nf_core/modules/module_utils.py @@ -1,7 +1,6 @@ import glob import json import os -import requests import logging import rich import datetime @@ -15,6 +14,8 @@ log = logging.getLogger(__name__) +gh_api = nf_core.utils.gh_api + class ModuleException(Exception): """Exception raised when there was an error with module commands""" @@ -35,7 +36,7 @@ def module_exist_in_repo(module_name, modules_repo): api_url = ( f"https://api.github.com/repos/{modules_repo.name}/contents/modules/{module_name}?ref={modules_repo.branch}" ) - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) return not (response.status_code == 404) @@ -65,7 +66,7 @@ def get_module_git_log(module_name, modules_repo=None, per_page=30, page_nbr=1, api_url += f"&since={since}" log.debug(f"Fetching commit history of module '{module_name}' from github API") - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 200: commits = response.json() @@ -80,6 +81,7 @@ def get_module_git_log(module_name, modules_repo=None, per_page=30, page_nbr=1, elif response.status_code == 404: raise LookupError(f"Module '{module_name}' not found in '{modules_repo.name}'\n{api_url}") else: + gh_api.log_content_headers(response) raise LookupError( f"Unable to fetch commit SHA for module {module_name}. API responded with '{response.status_code}'" ) @@ -101,7 +103,7 @@ def get_commit_info(commit_sha, repo_name="nf-core/modules"): ) api_url = f"https://api.github.com/repos/{repo_name}/commits/{commit_sha}?stats=false" log.debug(f"Fetching commit metadata for commit at {commit_sha}") - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 200: commit = response.json() message = commit["commit"]["message"].partition("\n")[0] @@ -115,6 +117,7 @@ def get_commit_info(commit_sha, repo_name="nf-core/modules"): elif response.status_code == 404: raise LookupError(f"Commit '{commit_sha}' not found in 'nf-core/modules/'\n{api_url}") else: + gh_api.log_content_headers(response) raise LookupError(f"Unable to fetch metadata for commit SHA {commit_sha}") @@ -266,10 +269,12 @@ def local_module_equal_to_commit(local_files, module_name, modules_repo, commit_ for i, file in enumerate(files_to_check): # Download remote copy and compare api_url = f"{module_base_url}/{file}" - r = nf_core.utils.call_github_api(api_url, raise_error=False) + r = gh_api.get(api_url) + # TODO: Remove debugging + gh_api.log_content_headers(r) if r.status_code != 200: + gh_api.log_content_headers(r) log.debug(f"Could not download remote copy of file module {module_name}/{file}") - log.debug(api_url) else: try: remote_copies[i] = r.content.decode("utf-8") @@ -414,7 +419,7 @@ def verify_pipeline_dir(dir): modules_is_software = False for repo_name in repo_names: api_url = f"https://api.github.com/repos/{repo_name}/contents" - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 404: missing_remote.append(repo_name) if repo_name == "nf-core/software": diff --git a/nf_core/modules/modules_repo.py b/nf_core/modules/modules_repo.py index 0fc7189d64..b4faba3cbc 100644 --- a/nf_core/modules/modules_repo.py +++ b/nf_core/modules/modules_repo.py @@ -1,9 +1,7 @@ import os -import requests import base64 -import sys import logging -import nf_core.utils +from nf_core.utils import gh_api log = logging.getLogger(__name__) @@ -43,7 +41,7 @@ def __init__(self, repo="nf-core/modules", branch=None): def get_default_branch(self): """Get the default branch for a GitHub repo""" api_url = f"https://api.github.com/repos/{self.name}" - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 200: self.branch = response.json()["default_branch"] log.debug(f"Found default branch to be '{self.branch}'") @@ -58,7 +56,7 @@ def verify_modules_repo(self): # Check if repository exist api_url = f"https://api.github.com/repos/{self.name}/branches" - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 200: branches = [branch["name"] for branch in response.json()] if self.branch not in branches: @@ -67,7 +65,7 @@ def verify_modules_repo(self): raise LookupError(f"Repository '{self.name}' is not available on GitHub") api_url = f"https://api.github.com/repos/{self.name}/contents?ref={self.branch}" - response = nf_core.utils.call_github_api(api_url, raise_error=False) + response = gh_api.get(api_url) if response.status_code == 200: dir_names = [entry["name"] for entry in response.json() if entry["type"] == "dir"] if "modules" not in dir_names: @@ -86,7 +84,7 @@ def get_modules_file_tree(self): self.modules_avail_module_names """ api_url = "https://api.github.com/repos/{}/git/trees/{}?recursive=1".format(self.name, self.branch) - r = nf_core.utils.call_github_api(api_url, raise_error=False) + r = gh_api.get(api_url) if r.status_code == 404: raise LookupError("Repository / branch not found: {} ({})\n{}".format(self.name, self.branch, api_url)) elif r.status_code != 200: @@ -157,7 +155,7 @@ def download_gh_file(self, dl_filename, api_url): os.makedirs(dl_directory) # Call the GitHub API - r = nf_core.utils.call_github_api(api_url, raise_error=False) + r = gh_api.get(api_url) if r.status_code != 200: raise LookupError("Could not fetch {} file: {}\n {}".format(self.name, r.status_code, api_url)) result = r.json() diff --git a/nf_core/sync.py b/nf_core/sync.py index d4452ecd2c..979430b9e3 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -6,13 +6,10 @@ import json import logging import os -import random -import re import requests import requests_cache import rich import shutil -import time import nf_core import nf_core.create @@ -318,9 +315,12 @@ def make_pull_request(self): # Make new pull-request stderr = rich.console.Console(stderr=True, force_terminal=nf_core.utils.rich_force_colors()) log.debug("Submitting PR to GitHub API") - with requests_cache.disabled(): + gh_api = nf_core.utils.gh_api + gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) + gh_api.return_ok = [201] + with gh_api.disabled(): try: - r = nf_core.utils.call_github_api( + r = gh_api.get_retry( f"https://api.github.com/repos/{self.gh_repo}/pulls", post_data={ "title": pr_title, @@ -329,9 +329,6 @@ def make_pull_request(self): "head": self.merge_branch, "base": self.from_branch, }, - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - return_ok=[201], - return_retry=[403], ) except Exception as e: stderr.print_exception() diff --git a/nf_core/utils.py b/nf_core/utils.py index a1dba30c6a..0b6c44d586 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -19,6 +19,7 @@ import re import requests import requests_cache +import rich import shlex import subprocess import sys @@ -296,21 +297,28 @@ def setup_requests_cachedir(): Caching directory will be set up in the user's home directory under a .nfcore_cache subdir. + + Uses requests_cache monkey patching. + Also returns the config dict so that we can use the same setup with a Session. """ pyversion = ".".join(str(v) for v in sys.version_info[0:3]) cachedir = os.path.join(os.getenv("HOME"), os.path.join(".nfcore", "cache_" + pyversion)) + config = { + "cache_name": os.path.join(cachedir, "github_info"), + "expire_after": datetime.timedelta(hours=1), + "backend": "sqlite", + } + try: if not os.path.exists(cachedir): os.makedirs(cachedir) - requests_cache.install_cache( - os.path.join(cachedir, "github_info"), - expire_after=datetime.timedelta(hours=1), - backend="sqlite", - ) + requests_cache.install_cache(**config) except PermissionError: pass + return config + def wait_cli_function(poll_func, poll_every=20): """ @@ -376,98 +384,145 @@ def poll_nfcore_web_api(api_url, post_data=None): return web_response -def call_github_api(url, post_data=None, return_ok=[200, 201], return_retry=[], auth=None, raise_error=True): +class GitHub_API_Session(requests_cache.CachedSession): + """ + Class to provide a single session for interacting with the GitHub API for a run. + Inherits the requests_cache.CachedSession and adds additional functionality, + such as automatically setting up GitHub authentication if we can. """ - Send a request to the GitHub API. - Uses authentication (to avoid rate limiting) if available. - Args: - url (str): The full URL of the GitHub API request. + def __init__(self): + self.auth_mode = None + self.return_ok = [200, 201] + self.return_retry = [403] + self.has_init = False - Returns: - data (dict): The JSON response from the GitHub API. - """ + def lazy_init(self): + """ + Initialise the object. + + Only do this when it's actually being used (due to global import) + """ + log.debug("Initialising GitHub API requests session") + cache_config = setup_requests_cachedir() + super().__init__(**cache_config) + self.setup_github_auth() + self.has_init = True - auth_mode = None - if auth is not None: - auth_mode = "supplied to function" + def setup_github_auth(self, auth=None): + """ + Try to automatically set up GitHub authentication + """ + if auth is not None: + self.auth = auth + self.auth_mode = "supplied to function" + + # Class for Bearer token authentication + # https://stackoverflow.com/a/58055668/713980 + class BearerAuth(requests.auth.AuthBase): + def __init__(self, token): + self.token = token + + def __call__(self, r): + r.headers["authorization"] = f"Bearer {self.token}" + return r + + # Default auth if we're running and the gh CLI tool is installed + gh_cli_config_fn = os.path.expanduser("~/.config/gh/hosts.yml") + if self.auth is None and os.path.exists(gh_cli_config_fn): + try: + with open(gh_cli_config_fn, "r") as fh: + gh_cli_config = yaml.safe_load(fh) + self.auth = requests.auth.HTTPBasicAuth( + gh_cli_config["github.com"]["user"], gh_cli_config["github.com"]["oauth_token"] + ) + self.auth_mode = f"gh CLI config: {gh_cli_config['github.com']['user']}" + except Exception as e: + output = rich.markup.escape(e.output.decode()) + log.debug(f"Couldn't auto-auth with GitHub CLI auth: [red]{output}") - # Class for Bearer token authentication - # https://stackoverflow.com/a/58055668/713980 - class BearerAuth(requests.auth.AuthBase): - def __init__(self, token): - self.token = token + # Default auth if we have a GitHub Token (eg. GitHub Actions CI) + if os.environ.get("GITHUB_TOKEN") is not None and self.auth is None: + self.auth_mode = "Bearer token with GITHUB_TOKEN" + self.auth = BearerAuth(os.environ["GITHUB_TOKEN"]) - def __call__(self, r): - r.headers["authorization"] = f"Bearer {self.token}" - return r + log.debug(f"Using GitHub auth: {self.auth_mode}") - # Default auth if we're running and the gh CLI tool is installed - gh_cli_config_fn = os.path.expanduser("~/.config/gh/hosts.yml") - if auth is None and os.path.exists(gh_cli_config_fn): + def log_content_headers(self, request): + """ + Try to dump everything to the console, useful when things go wrong. + """ + rich.inspect(request) try: - with open(gh_cli_config_fn, "r") as fh: - gh_cli_config = yaml.safe_load(fh) - auth = requests.auth.HTTPBasicAuth( - gh_cli_config["github.com"]["user"], gh_cli_config["github.com"]["oauth_token"] - ) - auth_mode = f"gh CLI config: {gh_cli_config['github.com']['user']}" + log.debug(json.dumps(dict(request.headers), indent=4)) + log.debug(json.dumps(request.json(), indent=4)) except Exception as e: - log.debug(f"Couldn't auto-auth with GitHub CLI auth: [red]{e}") + log.debug(f"Could not parse JSON response from GitHub API! {e}") + log.debug(request.headers) + log.debug(request.content) - # Default auth if we have a GitHub Token (eg. GitHub Actions CI) - if os.environ.get("GITHUB_TOKEN") is not None and auth is None: - auth_mode = "GITHUB_TOKEN" - auth = BearerAuth(os.environ["GITHUB_TOKEN"]) + def safe_get(self, url): + """ + Run a GET request, raise a nice exception with lots of logging if it fails. + """ + if not self.has_init: + self.lazy_init() + request = self.get(url) + if request.status_code not in self.return_ok: + self.log_content_headers(request) + raise AssertionError(f"GitHub API PR failed - got return code {request.status_code} from {url}") + return request + + def get(self, url, **kwargs): + """ + Initialise the session if we haven't already, then call the superclass get method. + """ + if not self.has_init: + self.lazy_init() + return super().get(url, **kwargs) - log.debug(f"Fetching GitHub API: {url} (auth: {auth_mode})") + def get_retry(self, url, post_data=None): + """ + Try to fetch a URL, keep retrying if we get a certain return code. - # Start the loop for a retry mechanism - while True: - # GET request - if post_data is None: - r = requests.get(url=url, auth=auth) - # POST request - else: - r = requests.post(url=url, json=post_data, auth=auth) + Used in nf-core sync code because we get 403 errors: too many simultaneous requests + See https://github.com/nf-core/tools/issues/911 + """ + if not self.has_init: + self.lazy_init() - try: - r_headers_pp = json.dumps(dict(r.headers), indent=4) - r_data_pp = json.dumps(r.json(), indent=4) - except Exception as e: - log.debug(r.headers) - log.debug(r.content) - if raise_error: - raise AssertionError(f"Could not parse JSON response from GitHub API! {e}") + # Start the loop for a retry mechanism + while True: + # GET request + if post_data is None: + r = self.get(url=url) + # POST request else: - return r - - # Failed but expected - try again - # Added due to 403 error: too many simultaneous requests - # See https://github.com/nf-core/tools/issues/911 - if r.status_code in return_retry: - log.debug(f"GitHub API PR failed - got return code {r.status_code}") - log.debug(r_headers_pp) - log.debug(r_data_pp) - wait_time = float(re.sub("[^0-9]", "", str(r.headers.get("Retry-After", 0)))) - if wait_time == 0: - log.debug("Couldn't find 'Retry-After' header, guessing a length of time to wait") - wait_time = random.randrange(10, 60) - log.warning(f"Got API return code {r.status_code}. Trying again after {wait_time} seconds..") - time.sleep(wait_time) - - # Unexpected error - raise - elif r.status_code not in return_ok: - log.debug(r_headers_pp) - log.debug(r_data_pp) - if raise_error: + r = self.post(url=url, json=post_data) + + # Failed but expected - try again + if r.status_code in self.return_retry: + self.log_content_headers(r) + log.debug(f"GitHub API PR failed - got return code {r.status_code}") + wait_time = float(re.sub("[^0-9]", "", str(r.headers.get("Retry-After", 0)))) + if wait_time == 0: + log.debug("Couldn't find 'Retry-After' header, guessing a length of time to wait") + wait_time = random.randrange(10, 60) + log.warning(f"Got API return code {r.status_code}. Trying again after {wait_time} seconds..") + time.sleep(wait_time) + + # Unexpected error - raise + elif r.status_code not in self.return_ok: + self.log_content_headers(r) raise AssertionError(f"GitHub API PR failed - got return code {r.status_code} from {url}") + + # Success! else: return r - # Success! - else: - return r + +# Single session object to use for entire codebase. Not sure if there's a better way to do this? +gh_api = GitHub_API_Session() def anaconda_package(dep, dep_channels=["conda-forge", "bioconda", "defaults"]): @@ -711,7 +766,7 @@ def prompt_remote_pipeline_name(wfs): else: if pipeline.count("/") == 1: try: - call_github_api(f"https://api.github.com/repos/{pipeline}") + gh_api.get(f"https://api.github.com/repos/{pipeline}") except Exception: # No repo found - pass and raise error at the end pass @@ -790,7 +845,7 @@ def get_repo_releases_branches(pipeline, wfs): ) # Get releases from GitHub API - rel_r = call_github_api(f"https://api.github.com/repos/{pipeline}/releases") + rel_r = gh_api.safe_get(f"https://api.github.com/repos/{pipeline}/releases") # Check that this repo existed try: @@ -804,7 +859,7 @@ def get_repo_releases_branches(pipeline, wfs): # Get release tag commit hashes if len(wf_releases) > 0: # Get commit hash information for each release - tags_r = call_github_api(f"https://api.github.com/repos/{pipeline}/tags") + tags_r = gh_api.safe_get(f"https://api.github.com/repos/{pipeline}/tags") for tag in tags_r.json(): for release in wf_releases: if tag["name"] == release["tag_name"]: @@ -815,7 +870,7 @@ def get_repo_releases_branches(pipeline, wfs): raise AssertionError(f"Not able to find pipeline '{pipeline}'") # Get branch information from github api - should be no need to check if the repo exists again - branch_response = call_github_api(f"https://api.github.com/repos/{pipeline}/branches") + branch_response = gh_api.safe_get(f"https://api.github.com/repos/{pipeline}/branches") for branch in branch_response.json(): if ( branch["name"] != "TEMPLATE" From 605dc8a0323ea16632492dd3c779560605e94ece Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 13 Apr 2022 16:31:10 +0200 Subject: [PATCH 62/93] Use new class more in the sync code --- nf_core/sync.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 979430b9e3..e62bea51a6 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -76,6 +76,11 @@ def __init__( self.gh_repo = gh_repo self.pr_url = "" + self.gh_api = nf_core.utils.gh_api + self.gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) + self.gh_api.return_ok = [201] + self.gh_api.lazy_init() + def sync(self): """Find workflow attributes, create a new template pipeline on TEMPLATE""" @@ -315,12 +320,9 @@ def make_pull_request(self): # Make new pull-request stderr = rich.console.Console(stderr=True, force_terminal=nf_core.utils.rich_force_colors()) log.debug("Submitting PR to GitHub API") - gh_api = nf_core.utils.gh_api - gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) - gh_api.return_ok = [201] - with gh_api.disabled(): + with self.gh_api.cache_disabled(): try: - r = gh_api.get_retry( + r = self.gh_api.get_retry( f"https://api.github.com/repos/{self.gh_repo}/pulls", post_data={ "title": pr_title, @@ -348,11 +350,8 @@ def close_open_template_merge_prs(self): # Look for existing pull-requests list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls" - with requests_cache.disabled(): - list_prs_request = requests.get( - url=list_prs_url, - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) + with self.gh_api.cache_disabled(): + list_prs_request = self.gh_api.get(list_prs_url) try: list_prs_json = json.loads(list_prs_request.content) list_prs_pp = json.dumps(list_prs_json, indent=4) @@ -390,20 +389,12 @@ def close_open_pr(self, pr): f"This pull-request is now outdated and has been closed in favour of {self.pr_url}\n\n" f"Please use {self.pr_url} to merge in the new changes from the nf-core template as soon as possible." ) - with requests_cache.disabled(): - comment_request = requests.post( - url=pr["comments_url"], - data=json.dumps({"body": comment_text}), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) + with self.gh_api.cache_disabled(): + self.gh_api.post(url=pr["comments_url"], data=json.dumps({"body": comment_text})) # Update the PR status to be closed - with requests_cache.disabled(): - pr_request = requests.patch( - url=pr["url"], - data=json.dumps({"state": "closed"}), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) + with self.gh_api.cache_disabled(): + pr_request = self.gh_api.patch(url=pr["url"], data=json.dumps({"state": "closed"})) try: pr_request_json = json.loads(pr_request.content) pr_request_pp = json.dumps(pr_request_json, indent=4) From 002deff62d27f7cc5d09fe3031b5ff9f1465caec Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 13 Apr 2022 17:09:18 +0200 Subject: [PATCH 63/93] Nearly fix the tests --- nf_core/sync.py | 5 +++-- nf_core/utils.py | 7 +++++-- tests/test_sync.py | 49 +++++++++++++++++++++++++++++++++------------- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index e62bea51a6..5960048ca1 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -77,7 +77,8 @@ def __init__( self.pr_url = "" self.gh_api = nf_core.utils.gh_api - self.gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) + if self.gh_username and "GITHUB_AUTH_TOKEN" in os.environ: + self.gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) self.gh_api.return_ok = [201] self.gh_api.lazy_init() @@ -322,7 +323,7 @@ def make_pull_request(self): log.debug("Submitting PR to GitHub API") with self.gh_api.cache_disabled(): try: - r = self.gh_api.get_retry( + r = self.gh_api.request_retry( f"https://api.github.com/repos/{self.gh_repo}/pulls", post_data={ "title": pr_title, diff --git a/nf_core/utils.py b/nf_core/utils.py index 0b6c44d586..38e8c96264 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -452,7 +452,10 @@ def log_content_headers(self, request): """ Try to dump everything to the console, useful when things go wrong. """ - rich.inspect(request) + log.debug(f"Requested URL: {request.url}") + log.debug(f"From requests cache: {request.from_cache}") + log.debug(f"Request status code: {request.status_code}") + log.debug(f"Request reason: {request.reason}") try: log.debug(json.dumps(dict(request.headers), indent=4)) log.debug(json.dumps(request.json(), indent=4)) @@ -481,7 +484,7 @@ def get(self, url, **kwargs): self.lazy_init() return super().get(url, **kwargs) - def get_retry(self, url, post_data=None): + def request_retry(self, url, post_data=None): """ Try to fetch a URL, keep retrying if we get a certain return code. diff --git a/tests/test_sync.py b/tests/test_sync.py index a0d009638f..42cfc87918 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -162,69 +162,90 @@ def test_push_template_branch_error(self): except nf_core.sync.PullRequestException as e: assert e.args[0].startswith("Could not push TEMPLATE branch") - def mocked_requests_get(**kwargs): + def mocked_requests_get(url, **kwargs): """Helper function to emulate POST requests responses from the web""" class MockResponse: def __init__(self, data, status_code): + self.url = kwargs.get("url") self.status_code = status_code + self.from_cache = False + self.reason = "Mocked response" + self.data = data self.content = json.dumps(data) + self.headers = {"content-encoding": "test", "connection": "fake"} + + def json(self): + return self.data url_template = "https://api.github.com/repos/{}/response/pulls?head=TEMPLATE&base=None" - if kwargs["url"] == url_template.format("no_existing_pr"): + if url == url_template.format("no_existing_pr"): response_data = [] return MockResponse(response_data, 200) - return MockResponse({"get_url": kwargs["url"]}, 404) + return MockResponse({"html_url": url}, 404) - def mocked_requests_patch(**kwargs): + def mocked_requests_patch(url, **kwargs): """Helper function to emulate POST requests responses from the web""" class MockResponse: def __init__(self, data, status_code): + self.url = kwargs.get("url") self.status_code = status_code + self.from_cache = False + self.reason = "Mocked" self.content = json.dumps(data) + self.headers = {"content-encoding": "test", "connection": "fake"} - if kwargs["url"] == "url_to_update_pr": + if url == "url_to_update_pr": response_data = {"html_url": "great_success"} return MockResponse(response_data, 200) - return MockResponse({"patch_url": kwargs["url"]}, 404) + return MockResponse({"patch_url": url}, 404) - def mocked_requests_post(**kwargs): + def mocked_requests_post(url, **kwargs): """Helper function to emulate POST requests responses from the web""" class MockResponse: def __init__(self, data, status_code): + self.url = kwargs.get("url") self.status_code = status_code + self.from_cache = False + self.reason = "Mocked" + self.data = data self.content = json.dumps(data) self.headers = {"content-encoding": "test", "connection": "fake"} def json(self): - return json.loads(self.content) + return self.data - if kwargs["url"] == "https://api.github.com/repos/no_existing_pr/response/pulls": + if url == "https://api.github.com/repos/no_existing_pr/response/pulls": response_data = {"html_url": "great_success"} return MockResponse(response_data, 201) - return MockResponse({"post_url": kwargs["url"]}, 404) + response_data = {} + return MockResponse(response_data, 404) - @mock.patch("requests.get", side_effect=mocked_requests_get) - @mock.patch("requests.post", side_effect=mocked_requests_post) + @mock.patch("nf_core.utils.gh_api.get", side_effect=mocked_requests_get) + @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) def test_make_pull_request_success(self, mock_get, mock_post): """Try making a PR - successful response""" psync = nf_core.sync.PipelineSync(self.pipeline_dir) + psync.gh_api.get = mock_get + psync.gh_api.post = mock_post psync.gh_username = "no_existing_pr" psync.gh_repo = "no_existing_pr/response" os.environ["GITHUB_AUTH_TOKEN"] = "test" psync.make_pull_request() assert psync.gh_pr_returned_data["html_url"] == "great_success" - @mock.patch("requests.get", side_effect=mocked_requests_get) - @mock.patch("requests.post", side_effect=mocked_requests_post) + @mock.patch("nf_core.utils.gh_api.get", side_effect=mocked_requests_get) + @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) def test_make_pull_request_bad_response(self, mock_get, mock_post): """Try making a PR and getting a 404 error""" psync = nf_core.sync.PipelineSync(self.pipeline_dir) + psync.gh_api.get = mock_get + psync.gh_api.post = mock_post psync.gh_username = "bad_url" psync.gh_repo = "bad_url/response" os.environ["GITHUB_AUTH_TOKEN"] = "test" From 4cbbee808d515d6576845383ff60b026112a3d9a Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 10 May 2022 10:04:02 +0200 Subject: [PATCH 64/93] Better exception handling / logging for gh auth failure --- nf_core/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nf_core/utils.py b/nf_core/utils.py index 38e8c96264..e8ed422dee 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -438,8 +438,9 @@ def __call__(self, r): ) self.auth_mode = f"gh CLI config: {gh_cli_config['github.com']['user']}" except Exception as e: - output = rich.markup.escape(e.output.decode()) - log.debug(f"Couldn't auto-auth with GitHub CLI auth: [red]{output}") + ex_type, ex_value, ex_traceback = sys.exc_info() + output = rich.markup.escape(f"{ex_type.__name__}: {ex_value}") + log.debug(f"Couldn't auto-auth with GitHub CLI auth from '{gh_cli_config_fn}': [red]{output}") # Default auth if we have a GitHub Token (eg. GitHub Actions CI) if os.environ.get("GITHUB_TOKEN") is not None and self.auth is None: From de429f7d0d69e66f1ab86631fc91fc679325365a Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 10 May 2022 10:13:14 +0200 Subject: [PATCH 65/93] Use XDG_CONFIG_HOME or ~/.config/nf-core/ for the requests cache dir. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suggested by @Emiller88 👍🏻 --- nf_core/utils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nf_core/utils.py b/nf_core/utils.py index e8ed422dee..d56d43ca64 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -49,6 +49,11 @@ ] ) +NFCORE_CONFIG_DIR = os.path.join( + os.environ.get("XDG_CONFIG_HOME", os.path.join(os.getenv("HOME"), ".config")), + "nf-core", +) + def check_if_outdated(current_version=None, remote_version=None, source_url="https://nf-co.re/tools_version"): """ @@ -296,13 +301,13 @@ def setup_requests_cachedir(): """Sets up local caching for faster remote HTTP requests. Caching directory will be set up in the user's home directory under - a .nfcore_cache subdir. + a .config/nf-core/cache_* subdir. Uses requests_cache monkey patching. Also returns the config dict so that we can use the same setup with a Session. """ pyversion = ".".join(str(v) for v in sys.version_info[0:3]) - cachedir = os.path.join(os.getenv("HOME"), os.path.join(".nfcore", "cache_" + pyversion)) + cachedir = os.path.join(NFCORE_CONFIG_DIR, f"cache_{pyversion}") config = { "cache_name": os.path.join(cachedir, "github_info"), From c0f179d042ffccee8af533e4b59a3be676f2b626 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 10 May 2022 10:17:42 +0200 Subject: [PATCH 66/93] resolve merge conflicts after rebase Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e72c06b67..6c0ec55c33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ - Add actions workflow to respond to `@nf-core-bot fix linting` comments on nf-core/tools PRs - Linting: Don't allow a `.nf-core.yaml` file, should be `.yml` ([#1515](https://github.com/nf-core/tools/pull/1515)). - Not all definitions in JSON schema have a "properties", leading to an error ([#1419](https://github.com/nf-core/tools/issues/1419)) +- Remove empty JSON schema definition groups to avoid usage errors ([#1419](https://github.com/nf-core/tools/issues/1419)) +- Print include statement to terminal when `modules install` ([#1520](https://github.com/nf-core/tools/pull/1520)) +- Use [`$XDG_CONFIG_HOME`](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) or `~/.config/nf-core` instead of `~/.nfcore` for API cache (the latter can be safely deleted) +- Consolidate GitHub API calls into a shared function that uses authentication from the [`gh` GitHub cli tool](https://cli.github.com/) or `GITHUB_AUTH_TOKEN` to avoid rate limiting ([#1499](https://github.com/nf-core/tools/pull/1499)) ### Modules From 1490a0b1408ad662210b70de6301340f463554c2 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 11 May 2022 15:03:55 +0200 Subject: [PATCH 67/93] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- tests/test_sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_sync.py b/tests/test_sync.py index 42cfc87918..66915009b8 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -228,7 +228,7 @@ def json(self): @mock.patch("nf_core.utils.gh_api.get", side_effect=mocked_requests_get) @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) - def test_make_pull_request_success(self, mock_get, mock_post): + def test_make_pull_request_success(self, mock_post, mock_get): """Try making a PR - successful response""" psync = nf_core.sync.PipelineSync(self.pipeline_dir) psync.gh_api.get = mock_get @@ -241,7 +241,7 @@ def test_make_pull_request_success(self, mock_get, mock_post): @mock.patch("nf_core.utils.gh_api.get", side_effect=mocked_requests_get) @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) - def test_make_pull_request_bad_response(self, mock_get, mock_post): + def test_make_pull_request_bad_response(self, mock_post, mock_get): """Try making a PR and getting a 404 error""" psync = nf_core.sync.PipelineSync(self.pipeline_dir) psync.gh_api.get = mock_get From 7227dad6a5600fb8c177c7eb0cfb75c6fa211c42 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Wed, 11 May 2022 15:04:43 +0200 Subject: [PATCH 68/93] Update nf_core/sync.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- nf_core/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 5960048ca1..e9f5ff391c 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -79,7 +79,7 @@ def __init__( self.gh_api = nf_core.utils.gh_api if self.gh_username and "GITHUB_AUTH_TOKEN" in os.environ: self.gh_api.auth = requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]) - self.gh_api.return_ok = [201] + self.gh_api.return_ok = [200, 201] self.gh_api.lazy_init() def sync(self): From 6ef8255b9fb56dffadad04642cc462addec75021 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 12 May 2022 09:50:03 +0200 Subject: [PATCH 69/93] Fix wrong merge conflict resolution --- nf_core/__main__.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 503e6b997d..45fcff9a60 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -706,25 +706,6 @@ def mulled(specifications, build_number): "If it does not, please add your desired combination as detailed at:\n" "https://github.com/BioContainers/multi-package-containers\n" ) - - -# nf-core modules test -@modules.command("test") -@click.pass_context -@click.argument("tool", type=str, required=False, metavar=" or ") -@click.option("-p", "--no-prompts", is_flag=True, default=False, help="Use defaults without prompting") -@click.option("-a", "--pytest_args", type=str, required=False, multiple=True, help="Additional pytest arguments") -def test_module(ctx, tool, no_prompts, pytest_args): - """ - Run module tests locally. - - Given the name of a module, runs the Nextflow test command. - """ - try: - meta_builder = nf_core.modules.ModulesTest(tool, no_prompts, pytest_args) - meta_builder.run() - except UserWarning as e: - log.critical(e) sys.exit(1) From 3dfbaccb4899e996fdd335ca326fea2175c568e2 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 12 May 2022 10:13:33 +0200 Subject: [PATCH 70/93] if-else instead of if-if --- nf_core/modules/update.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index b6c5a99c36..fa0f9af76a 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -287,17 +287,16 @@ def update(self, module): log.info(f"'{modules_repo.name}/{module}' is already up to date") continue - if not dry_run: + if dry_run: + # Set the install folder to a temporary directory + install_folder = [tempfile.mkdtemp()] + else: log.info(f"Updating '{modules_repo.name}/{module}'") log.debug(f"Updating module '{module}' to {version} from {modules_repo.name}") log.debug(f"Removing old version of module '{module}'") self.clear_module_dir(module, module_dir) - if dry_run: - # Set the install folder to a temporary directory - install_folder = [tempfile.mkdtemp()] - # Download module files if not self.download_module_file(module, version, modules_repo, install_folder, dry_run=dry_run): exit_value = False From b60a42410ae54d4ff7ac1f81db5999aa9b942c04 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 12 May 2022 10:16:21 +0200 Subject: [PATCH 71/93] collect more parameter checks --- nf_core/modules/update.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index fa0f9af76a..f1dc2d8a36 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -33,8 +33,9 @@ def __init__( self.update_all = update_all self.show_diff = show_diff self.save_diff_fn = save_diff_fn + self.module = None - def _parameter_compatibility_check(self): + def _parameter_checks(self): """Check the compatibilty of the supplied parameters. Checks: @@ -44,22 +45,26 @@ def _parameter_compatibility_check(self): if self.save_diff_fn and self.show_diff: raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") - def update(self, module): - - self._parameter_compatibility_check() if self.update_all and self.module: raise UserWarning("Either a module or the '--all' flag can be specified, not both.") if self.repo_type == "modules": raise UserWarning("Modules in clones of nf-core/modules can not be updated.") + if self.prompt and self.sha is not None: + raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") + if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") + def update(self, module): + + self.module = module + + self._parameter_checks() + # Verify that 'modules.json' is consistent with the installed modules self.modules_json_up_to_date() - if self.prompt and self.sha is not None: - raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") tool_config = nf_core.utils.load_tools_config() update_config = tool_config.get("update", {}) From caad09616333f514bbe91c2f45832711994b9d99 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Wed, 1 Jun 2022 11:57:26 +0200 Subject: [PATCH 72/93] fix missing import after merge conflict resolution --- nf_core/modules/update.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 34cabb0007..dab15cf7b9 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -18,6 +18,7 @@ get_installed_modules, get_module_git_log, module_exist_in_repo, + sha_exists, ) from .modules_command import ModuleCommand from .modules_repo import ModulesRepo From 1c06c0332e2c974b30a9460b7976cf411542a191 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 11:10:33 +0200 Subject: [PATCH 73/93] Fix failing test due to merge --- nf_core/modules/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index c06cdf6d2b..07b0351b4c 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -66,7 +66,7 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") - def update(self, module): + def update(self, module=None): self.module = module From bdee1d55de881f775303681d118763ecfca5142e Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 12:00:34 +0200 Subject: [PATCH 74/93] Move --all or not code to methods --- nf_core/modules/update.py | 281 ++++++++++++++++++++++---------------- nf_core/utils.py | 6 + 2 files changed, 172 insertions(+), 115 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 07b0351b4c..1925b05f04 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -12,6 +12,7 @@ import nf_core.modules.module_utils import nf_core.utils +from nf_core.utils import plural_s, plural_y from .modules_command import ModuleCommand from .modules_json import ModulesJson @@ -43,6 +44,8 @@ def __init__( self.show_diff = show_diff self.save_diff_fn = save_diff_fn self.module = None + self.update_config = None + self.module_json = None def _parameter_checks(self): """Check the compatibilty of the supplied parameters. @@ -66,18 +69,167 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") + def get_single_module_info(self, module): + """ + Get modules repo and module verion info for a single module. + + Information about the module version in the '.nf-core.yml' overrides + the '--sha' option + + Args: + module_name (str): The name of the module to get info for. + + Returns: + (ModulesRepo, str, str): The modules repo containing the module, + the module name, and the module version. + + Raises: + LookupError: If the module is not found either in the pipeline or the modules repo. + UserWarning: If the '.nf-core.yml' entry is not valid. + """ + # Check if there are any modules installed from the repo + repo_name = self.modules_repo.fullname + if repo_name not in self.module_names: + raise LookupError(f"No modules installed from '{repo_name}'") + + if module is None: + module = questionary.autocomplete( + "Tool name:", + choices=self.module_names[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]: + 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 + if module and module not in self.modules_repo.get_avail_modules(): + raise LookupError( + f"Module '{module}' not found in list of available modules." + f"Use the command 'nf-core modules list remote' to view available software" + ) + + sha = self.sha + if module in self.update_config.get(self.modules_repo.fullname, {}): + config_entry = self.update_config[self.modules_repo.fullname].get(module) + if config_entry is not None and config_entry is not True: + if config_entry is False: + raise UserWarning("Module's update entry in '.nf-core.yml' is set to False") + if not isinstance(config_entry, str): + raise UserWarning("Module's update entry in '.nf-core.yml' is of wrong type") + + sha = config_entry + if self.sha is not None: + log.warning( + f"Found entry in '.nf-core.yml' for module '{module}' " + "which will override version specified with '--sha'" + ) + else: + log.info(f"Found entry in '.nf-core.yml' for module '{module}'") + log.info(f"Updating module to ({sha})") + + return (self.modules_repo, module, sha) + + def get_all_modules_info(self): + """ + Get modules repo and module version information for all modules. + + Information about the module version in the '.nf-core.yml' overrides + the '--sha' option + + Returns: + [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object, + the module name, and the module version. + """ + skipped_repos = [] + skipped_modules = [] + overridden_repos = [] + overridden_modules = [] + repos_mods_shas = {} + # 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(): + if repo_name not in self.update_config or self.update_config[repo_name] is True: + repos_mods_shas[repo_name] = [(module, self.sha) for module in modules] + elif isinstance(self.update_config[repo_name], dict): + # If it is a dict, then there are entries for individual modules + repo_config = self.update_config[repo_name] + repos_mods_shas[repo_name] = [] + for module in modules: + if module not in repo_config or repo_config[module] is True: + repos_mods_shas[repo_name].append((module, self.sha)) + elif isinstance(repo_config[module], str): + # If a string is given it is the commit SHA to which we should update to + custom_sha = repo_config[module] + repos_mods_shas[repo_name].append((module, custom_sha)) + if self.sha is not None: + overridden_modules.append(module) + elif repo_config[module] is False: + # Otherwise the entry must be 'False' and we should ignore the module + skipped_modules.append(f"{repo_name}/{module}") + else: + raise UserWarning(f"Module '{module}' in '{repo_name}' has an invalid entry in '.nf-core.yml'") + elif isinstance(self.update_config[repo_name], str): + # If a string is given it is the commit SHA to which we should update to + custom_sha = self.update_config[repo_name] + repos_mods_shas[repo_name] = [(module_name, custom_sha) for module_name in modules] + if self.sha is not None: + overridden_repos.append(repo_name) + elif self.update_config[repo_name] is False: + skipped_repos.append(repo_name) + else: + raise UserWarning(f"Repo '{repo_name}' has an invalid entry in '.nf-core.yml'") + + if skipped_repos: + skipped_str = "', '".join(skipped_repos) + log.info(f"Skipping modules in repositor{plural_y(skipped_repos)}: '{skipped_str}'") + + if skipped_modules: + skipped_str = "', '".join(skipped_modules) + log.info(f"Skipping module{plural_s(skipped_modules)}: '{skipped_str}'") + + if overridden_repos: + overridden_str = "', '".join(overridden_repos) + log.info( + f"Overriding '--sha' flag for modules in repositor{plural_y(overridden_repos)} " + f"with '.nf-core.yml' entry: '{overridden_str}'" + ) + + if overridden_modules: + overridden_str = "', '".join(overridden_modules) + log.info( + f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with '.nf-core.yml' entry: '{overridden_str}'" + ) + + # Get the git urls from the modules.json + repos_mods_shas = [ + (self.modules_json.get_git_url(repo_name), self.modules_json.get_base_path(repo_name), mods_shas) + for repo_name, mods_shas in repos_mods_shas.items() + ] + + # Create ModulesRepo objects + repos_mods_shas = [ + (ModulesRepo(remote_url=repo_url, base_path=base_path), mods_shas) + for repo_url, base_path, mods_shas in repos_mods_shas + ] + + # Flatten the list + repos_mods_shas = [(repo, mod, sha) for repo, mods_shas in repos_mods_shas for mod, sha in mods_shas] + def update(self, module=None): self.module = module + tool_config = nf_core.utils.load_tools_config(self.dir) + self.update_config = tool_config.get("update", {}) + self._parameter_checks() # Verify that 'modules.json' is consistent with the installed modules - modules_json = ModulesJson(self.dir) - modules_json.check_up_to_date() + self.modules_json = ModulesJson(self.dir) + self.modules_json.check_up_to_date() - tool_config = nf_core.utils.load_tools_config(self.dir) - update_config = tool_config.get("update", {}) if not self.update_all and module is None: choices = ["All modules", "Named module"] self.update_all = ( @@ -90,116 +242,15 @@ def update(self, module=None): ) # Verify that the provided SHA exists in the repo - if self.sha: - if not self.modules_repo.sha_exists_on_branch(self.sha): - log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'") - return False - - if not self.update_all: - self.get_pipeline_modules() - - # Check if there are any modules installed from the repo - repo_name = self.modules_repo.fullname - if repo_name not in self.module_names: - log.error(f"No modules installed from '{repo_name}'") - return False - - if module is None: - module = questionary.autocomplete( - "Tool name:", - choices=self.module_names[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]: - log.error(f"Module '{module}' is not installed in pipeline and could therefore not be updated") - return False - - sha = self.sha - if module in update_config.get(self.modules_repo.fullname, {}): - config_entry = update_config[self.modules_repo.fullname].get(module) - if config_entry is not None and config_entry is not True: - if config_entry is False: - log.info("Module's update entry in '.nf-core.yml' is set to False") - return False - if not isinstance(config_entry, str): - log.error("Module's update entry in '.nf-core.yml' is of wrong type") - return False - - sha = config_entry - if self.sha: - log.warning( - f"Found entry in '.nf-core.yml' for module '{module}' " - "which will override version specified with '--sha'" - ) - else: - log.info(f"Found entry in '.nf-core.yml' for module '{module}'") - log.info(f"Updating module to ({sha})") - - # Check that the supplied name is an available module - if module and module not in self.modules_repo.get_avail_modules(): - log.error(f"Module '{module}' not found in list of available modules.") - log.info("Use the command 'nf-core modules list remote' to view available software") - return False - - repos_mods_shas = [(self.modules_repo, module, sha)] - - else: - - self.get_pipeline_modules() - - # Filter out modules that should not be updated or assign versions if there are any - skipped_repos = [] - skipped_modules = [] - repos_mods_shas = {} - for repo_name, modules in self.module_names.items(): - if repo_name not in update_config or update_config[repo_name] is True: - repos_mods_shas[repo_name] = [(module, self.sha) for module in modules] - elif isinstance(update_config[repo_name], dict): - repo_config = update_config[repo_name] - repos_mods_shas[repo_name] = [] - for module in modules: - if module not in repo_config or repo_config[module] is True: - repos_mods_shas[repo_name].append((module, self.sha)) - elif isinstance(repo_config[module], str): - # If a string is given it is the commit SHA to which we should update to - custom_sha = repo_config[module] - repos_mods_shas[repo_name].append((module, custom_sha)) - else: - # Otherwise the entry must be 'False' and we should ignore the module - skipped_modules.append(f"{repo_name}/{module}") - elif isinstance(update_config[repo_name], str): - # If a string is given it is the commit SHA to which we should update to - custom_sha = update_config[repo_name] - repos_mods_shas[repo_name] = [(module_name, custom_sha) for module_name in modules] - else: - skipped_repos.append(repo_name) - - if skipped_repos: - skipped_str = "', '".join(skipped_repos) - log.info(f"Skipping modules in repositor{'y' if len(skipped_repos) == 1 else 'ies'}: '{skipped_str}'") - - if skipped_modules: - skipped_str = "', '".join(skipped_modules) - log.info(f"Skipping module{'' if len(skipped_modules) == 1 else 's'}: '{skipped_str}'") - - # Get the git urls from the modules.json - repos_mods_shas = [ - (modules_json.get_git_url(repo_name), modules_json.get_base_path(repo_name), mods_shas) - for repo_name, mods_shas in repos_mods_shas.items() - ] - - repos_mods_shas = [ - (ModulesRepo(remote_url=repo_url, base_path=base_path), mods_shas) - for repo_url, base_path, mods_shas in repos_mods_shas - ] + if self.sha and not self.modules_repo.sha_exists_on_branch(self.sha): + log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'") + return False - # Flatten the list - repos_mods_shas = [(repo, mod, sha) for repo, mods_shas in repos_mods_shas for mod, sha in mods_shas] + self.get_pipeline_modules() + repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info()] # Save the current state of the modules.json - old_modules_json = modules_json.get_modules_json() + old_modules_json = self.modules_json.get_modules_json() # Ask if we should show the diffs (unless a filename was already given on the command line) if not self.save_diff_fn and self.show_diff is None: @@ -248,7 +299,7 @@ def update(self, module=None): exit_value = False continue - current_version = modules_json.get_module_version(module, modules_repo.fullname) + current_version = self.modules_json.get_module_version(module, modules_repo.fullname) # Set the install folder based on the repository name install_folder = [self.dir, "modules"] @@ -425,17 +476,17 @@ class DiffEnum(enum.Enum): # Update modules.json with newly installed module if not dry_run: - modules_json.update(modules_repo, module, version) + self.modules_json.update(modules_repo, module, version) # Don't save to a file, just iteratively update the variable else: - modules_json.update(modules_repo, module, version, write_file=False) + self.modules_json.update(modules_repo, module, version, write_file=False) if self.save_diff_fn: # Compare the new modules.json and build a diff modules_json_diff = difflib.unified_diff( json.dumps(old_modules_json, indent=4).splitlines(keepends=True), - json.dumps(modules_json.get_modules_json(), indent=4).splitlines(keepends=True), + json.dumps(self.modules_json.get_modules_json(), indent=4).splitlines(keepends=True), fromfile=os.path.join(self.dir, "modules.json"), tofile=os.path.join(self.dir, "modules.json"), ) diff --git a/nf_core/utils.py b/nf_core/utils.py index 822d5a6811..1bc47413dc 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -966,3 +966,9 @@ def plural_s(list_or_int): """Return an s if the input is not one or has not the length of one.""" length = list_or_int if isinstance(list_or_int, int) else len(list_or_int) return "s" * (length != 1) + + +def plural_y(list_or_int): + """Return 'ies' if the input is not one or has not the length of one, else 'y'.""" + length = list_or_int if isinstance(list_or_int, int) else len(list_or_int) + return "ies" if length != 1 else "y" From ea61cb3b87b7bafb7d809d631a925b101daf2965 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 12:42:27 +0200 Subject: [PATCH 75/93] Move more stuff to methods --- nf_core/modules/modules_repo.py | 6 +++ nf_core/modules/update.py | 71 ++++++++++++++++----------------- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/nf_core/modules/modules_repo.py b/nf_core/modules/modules_repo.py index 8e0c4771b5..9f7783c3a0 100644 --- a/nf_core/modules/modules_repo.py +++ b/nf_core/modules/modules_repo.py @@ -338,6 +338,12 @@ def get_module_git_log(self, module_name, depth=None, since="2021-07-07T00:00:00 commits = ({"git_sha": commit.hexsha, "trunc_message": commit.message.partition("\n")[0]} for commit in commits) return commits + def get_latest_module_version(self, module_name): + """ + Returns the latest commit in the repository + """ + return list(self.get_module_git_log(module_name, depth=1))[0]["git_sha"] + def sha_exists_on_branch(self, sha): """ Verifies that a given commit sha exists on the branch diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 1925b05f04..7b2f42256e 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -214,8 +214,24 @@ def get_all_modules_info(self): for repo_url, base_path, mods_shas in repos_mods_shas ] - # Flatten the list - repos_mods_shas = [(repo, mod, sha) for repo, mods_shas in repos_mods_shas for mod, sha in mods_shas] + # Flatten and return the list + return [(repo, mod, sha) for repo, mods_shas in repos_mods_shas for mod, sha in mods_shas] + + def setup_diff_file(self): + if self.save_diff_fn is True: + # From questionary - no filename yet + self.save_diff_fn = questionary.text( + "Enter the filename: ", style=nf_core.utils.nfcore_question_style + ).unsafe_ask() + # Check if filename already exists (questionary or cli) + while os.path.exists(self.save_diff_fn): + if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): + os.remove(self.save_diff_fn) + break + self.save_diff_fn = questionary.text( + "Enter a new filename: ", + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() def update(self, module=None): @@ -269,20 +285,7 @@ def update(self, module=None): # Set up file to save diff if self.save_diff_fn: # True or a string - # From questionary - no filename yet - if self.save_diff_fn is True: - self.save_diff_fn = questionary.text( - "Enter the filename: ", style=nf_core.utils.nfcore_question_style - ).unsafe_ask() - # Check if filename already exists (questionary or cli) - while os.path.exists(self.save_diff_fn): - if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): - os.remove(self.save_diff_fn) - break - self.save_diff_fn = questionary.text( - "Enter a new filename: ", - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() + self.setup_diff_file() exit_value = True for modules_repo, module, sha in repos_mods_shas: @@ -301,31 +304,28 @@ def update(self, module=None): current_version = self.modules_json.get_module_version(module, modules_repo.fullname) - # Set the install folder based on the repository name - install_folder = [self.dir, "modules"] - install_folder.extend(os.path.split(modules_repo.fullname)) + # Set the install folder based + repo_path = [self.dir, "modules"] + repo_path.extend(os.path.split(modules_repo.fullname)) + if not dry_run: + install_folder = repo_path + else: + install_folder = [tempfile.mkdtemp()] # Compute the module directory - module_dir = os.path.join(*install_folder, module) + module_dir = os.path.join(*repo_path, module) - if sha: + if sha is not None: version = sha elif self.prompt: - try: - version = nf_core.modules.module_utils.prompt_module_version_sha( - module, modules_repo=modules_repo, installed_sha=current_version - ) - except SystemError as e: - log.error(e) - exit_value = False - continue + version = nf_core.modules.module_utils.prompt_module_version_sha( + module, modules_repo=modules_repo, installed_sha=current_version + ) else: - # Fetch the latest commit for the module - git_log = list(modules_repo.get_module_git_log(module, depth=1)) - version = git_log[0]["git_sha"] + # Default to the latest commit for the module + version = modules_repo.get_latest_module_version(module) if current_version is not None and not self.force: - # Fetch the latest commit for the module if current_version == version: if self.sha or self.prompt: log.info(f"'{modules_repo.fullname}/{module}' is already installed at {version}") @@ -333,10 +333,7 @@ def update(self, module=None): log.info(f"'{modules_repo.fullname}/{module}' is already up to date") continue - if dry_run: - # Set the install folder to a temporary directory - install_folder = [tempfile.mkdtemp()] - else: + if not dry_run: log.info(f"Updating '{modules_repo.fullname}/{module}'") log.debug(f"Updating module '{module}' to {version} from {modules_repo.remote_url}") From fe42c54eb1c9bebe3b0a8f4a6d901b3363555501 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 13:38:44 +0200 Subject: [PATCH 76/93] Refactor dry run stuff into methods --- nf_core/modules/update.py | 294 +++++++++++++++++++++++--------------- 1 file changed, 179 insertions(+), 115 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 7b2f42256e..171ce03762 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -47,6 +47,19 @@ def __init__( self.update_config = None self.module_json = None + class DiffEnum(enum.Enum): + """ + Enumeration for keeping track of + the diff status of a pair of files + + Used for the --save-diff and --preview options + """ + + UNCHANGED = enum.auto() + CHANGED = enum.auto() + CREATED = enum.auto() + REMOVED = enum.auto() + def _parameter_checks(self): """Check the compatibilty of the supplied parameters. @@ -233,8 +246,166 @@ def setup_diff_file(self): style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - def update(self, module=None): + def get_module_diffs(self, install_folder, module, module_dir): + """ + Compute the diff between the current module version + and the new version. + + Args: + install_folder ([str]): The folder where the module is installed + module (str): The module name + module_dir (str): The directory containing the current version of the module + + Returns: + dict[str, (ModuleUpdate.DiffEnum, str)]: A dictionary containing + the diff type and the diff string (empty if no diff) + """ + + diffs = {} + # Get all unique filenames in the two folders. + # `dict.fromkeys()` is used instead of `set()` to preserve order + files = dict.fromkeys(os.listdir(os.path.join(*install_folder, module))) + files.update(dict.fromkeys(os.listdir(module_dir))) + files = list(files) + + temp_folder = os.path.join(*install_folder, module) + + # Loop through all the module files and compute their diffs if needed + for file in files: + temp_path = os.path.join(temp_folder, file) + curr_path = os.path.join(module_dir, file) + if os.path.exists(temp_path) and os.path.exists(curr_path) and os.path.isfile(temp_path): + with open(temp_path, "r") as fh: + new_lines = fh.readlines() + with open(curr_path, "r") as fh: + old_lines = fh.readlines() + + if new_lines == old_lines: + # The files are identical + diffs[file] = (ModuleUpdate.DiffEnum.UNCHANGED, ()) + else: + # Compute the diff + diff = difflib.unified_diff( + old_lines, + new_lines, + fromfile=os.path.join(module_dir, file), + tofile=os.path.join(module_dir, file), + ) + diffs[file] = (ModuleUpdate.DiffEnum.CHANGED, diff) + + elif os.path.exists(temp_path): + # The file was created + diffs[file] = (ModuleUpdate.DiffEnum.CREATED, ()) + + elif os.path.exists(curr_path): + # The file was removed + diffs[file] = (ModuleUpdate.DiffEnum.REMOVED, ()) + + return diffs + + def append_diff_file(self, module, diffs, module_dir, current_version, new_version): + """ + Writes the diffs of a module to the diff file. + + Args: + module (str): The module name + diffs (dict[str, (ModuleUpdate.DiffEnum, str)]): A dictionary containing + the type of change and the diff (if any) + module_dir (str): The path to the current installation of the module + current_version (str): The installed version of the module + new_version (str): The version of the module the diff is computed against + """ + log.info(f"Writing diff of '{module}' to '{self.save_diff_fn}'") + with open(self.save_diff_fn, "a") as fh: + fh.write( + f"Changes in module '{module}' between" + f" ({current_version if current_version is not None else '?'}) and" + f" ({new_version if new_version is not None else 'latest'})\n" + ) + + for file, (diff_status, diff) in diffs.items(): + if diff_status == ModuleUpdate.DiffEnum.UNCHANGED: + # The files are identical + fh.write(f"'{os.path.join(module_dir, file)}' is unchanged\n") + + elif diff_status == ModuleUpdate.DiffEnum.CREATED: + # The file was created between the commits + fh.write(f"'{os.path.join(module_dir, file)}' was created\n") + + elif diff_status == ModuleUpdate.DiffEnum.REMOVED: + # The file was removed between the commits + fh.write(f"'{os.path.join(module_dir, file)}' was removed\n") + + else: + # The file has changed + fh.write(f"Changes in '{os.path.join(module_dir, file)}':\n") + # Write the diff lines to the file + for line in diff: + fh.write(line) + fh.write("\n") + fh.write("*" * 60 + "\n") + + def print_diff(self, module, diffs, module_dir, current_version, new_version): + """ + Prints the diffs between two module versions + + Args: + module (str): The module name + diffs (dict[str, (ModuleUpdate.DiffEnum, str)]): A dictionary containing + the type of change and the diff (if any) + module_dir (str): The path to the current installation of the module + current_version (str): The installed version of the module + new_version (str): The version of the module the diff is computed against + """ + console = Console(force_terminal=nf_core.utils.rich_force_colors()) + log.info( + f"Changes in module '{module}' between" + f" ({current_version if current_version is not None else '?'}) and" + f" ({new_version if new_version is not None else 'latest'})" + ) + + for file, (diff_status, diff) in diffs.items(): + if diff_status == ModuleUpdate.DiffEnum.UNCHANGED: + # The files are identical + log.info(f"'{os.path.join(module_dir, file)}' is unchanged") + elif diff_status == ModuleUpdate.DiffEnum.CREATED: + # The file was created between the commits + log.info(f"'{os.path.join(module_dir, file)}' was created") + elif diff_status == ModuleUpdate.DiffEnum.REMOVED: + # The file was removed between the commits + log.info(f"'{os.path.join(module_dir, file)}' was removed") + else: + # The file has changed + log.info(f"Changes in '{os.path.join(module, file)}':") + # Pretty print the diff using the pygments diff lexer + console.print(Syntax("".join(diff), "diff", theme="ansi_light")) + + def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_repo, new_version): + """ + Move the files from the temporary installation directory to the + module directory. + + Args: + module (str): The module name + module_dir (str): The path to the module directory + install_folder [str]: The path to the temporary installation directory + modules_repo (ModulesRepo): The ModulesRepo object from which the module + was installed + new_version (str): The version of the module that was installed + """ + temp_module_dir = os.path.join(*install_folder, module) + files = os.listdir(temp_module_dir) + self.clear_module_dir(module, module_dir) + os.makedirs(module_dir) + for file in files: + path = os.path.join(temp_module_dir, file) + if os.path.exists(path): + shutil.move(path, os.path.join(module_dir, file)) + log.info(f"Updating '{modules_repo.fullname}/{module}'") + log.debug(f"Updating module '{module}' to {new_version} from {modules_repo.fullname}") + + def update(self, module=None): self.module = module tool_config = nf_core.utils.load_tools_config(self.dir) @@ -263,7 +434,8 @@ def update(self, module=None): return False self.get_pipeline_modules() - repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info()] + # Get the list of modules to update, and their version information + repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] # Save the current state of the modules.json old_modules_json = self.modules_json.get_modules_json() @@ -346,113 +518,12 @@ def update(self, module=None): continue if dry_run: - - class DiffEnum(enum.Enum): - """ - Enumeration for keeping track of - the diff status of a pair of files - """ - - UNCHANGED = enum.auto() - CHANGED = enum.auto() - CREATED = enum.auto() - REMOVED = enum.auto() - - diffs = {} - - # Get all unique filenames in the two folders. - # `dict.fromkeys()` is used instead of `set()` to preserve order - files = dict.fromkeys(os.listdir(os.path.join(*install_folder, module))) - files.update(dict.fromkeys(os.listdir(module_dir))) - files = list(files) - - temp_folder = os.path.join(*install_folder, module) - - # Loop through all the module files and compute their diffs if needed - for file in files: - temp_path = os.path.join(temp_folder, file) - curr_path = os.path.join(module_dir, file) - if os.path.exists(temp_path) and os.path.exists(curr_path) and os.path.isfile(temp_path): - with open(temp_path, "r") as fh: - new_lines = fh.readlines() - with open(curr_path, "r") as fh: - old_lines = fh.readlines() - - if new_lines == old_lines: - # The files are identical - diffs[file] = (DiffEnum.UNCHANGED, ()) - else: - # Compute the diff - diff = difflib.unified_diff( - old_lines, - new_lines, - fromfile=os.path.join(module_dir, file), - tofile=os.path.join(module_dir, file), - ) - diffs[file] = (DiffEnum.CHANGED, diff) - - elif os.path.exists(temp_path): - # The file was created - diffs[file] = (DiffEnum.CREATED, ()) - - elif os.path.exists(curr_path): - # The file was removed - diffs[file] = (DiffEnum.REMOVED, ()) - + # Compute the diffs for the module + diffs = self.get_module_diffs(install_folder, module, module_dir) if self.save_diff_fn: - log.info(f"Writing diff of '{module}' to '{self.save_diff_fn}'") - with open(self.save_diff_fn, "a") as fh: - fh.write( - f"Changes in module '{module}' between" - f" ({current_version if current_version is not None else '?'}) and" - f" ({version if version is not None else 'latest'})\n" - ) - - for file, (diff_status, diff) in diffs.items(): - if diff_status == DiffEnum.UNCHANGED: - # The files are identical - fh.write(f"'{os.path.join(module_dir, file)}' is unchanged\n") - - elif diff_status == DiffEnum.CREATED: - # The file was created between the commits - fh.write(f"'{os.path.join(module_dir, file)}' was created\n") - - elif diff_status == DiffEnum.REMOVED: - # The file was removed between the commits - fh.write(f"'{os.path.join(module_dir, file)}' was removed\n") - - else: - # The file has changed - fh.write(f"Changes in '{os.path.join(module_dir, file)}':\n") - # Write the diff lines to the file - for line in diff: - fh.write(line) - fh.write("\n") - - fh.write("*" * 60 + "\n") + self.append_diff_file(module, diffs, module_dir, current_version, version) elif self.show_diff: - console = Console(force_terminal=nf_core.utils.rich_force_colors()) - log.info( - f"Changes in module '{module}' between" - f" ({current_version if current_version is not None else '?'}) and" - f" ({version if version is not None else 'latest'})" - ) - - for file, (diff_status, diff) in diffs.items(): - if diff_status == DiffEnum.UNCHANGED: - # The files are identical - log.info(f"'{os.path.join(module, file)}' is unchanged") - elif diff_status == DiffEnum.CREATED: - # The file was created between the commits - log.info(f"'{os.path.join(module, file)}' was created") - elif diff_status == DiffEnum.REMOVED: - # The file was removed between the commits - log.info(f"'{os.path.join(module, file)}' was removed") - else: - # The file has changed - log.info(f"Changes in '{os.path.join(module, file)}':") - # Pretty print the diff using the pygments diff lexer - console.print(Syntax("".join(diff), "diff", theme="ansi_light")) + self.print_diff(module, diffs, module_dir, current_version, version) # Ask the user if they want to install the module dry_run = not questionary.confirm( @@ -462,14 +533,7 @@ class DiffEnum(enum.Enum): # The new module files are already installed. # We just need to clear the directory and move the # new files from the temporary directory - self.clear_module_dir(module, module_dir) - os.makedirs(module_dir) - for file in files: - path = os.path.join(temp_folder, file) - if os.path.exists(path): - shutil.move(path, os.path.join(module_dir, file)) - log.info(f"Updating '{modules_repo.fullname}/{module}'") - log.debug(f"Updating module '{module}' to {version} from {modules_repo.fullname}") + self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) # Update modules.json with newly installed module if not dry_run: From d51e0970865be14aa0980cc6032ad306643b4a74 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 13:43:30 +0200 Subject: [PATCH 77/93] Always move files to temp dir first, to make sure the installation works before removing files --- nf_core/modules/update.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 171ce03762..6e7f7ecd19 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -479,10 +479,7 @@ def update(self, module=None): # Set the install folder based repo_path = [self.dir, "modules"] repo_path.extend(os.path.split(modules_repo.fullname)) - if not dry_run: - install_folder = repo_path - else: - install_folder = [tempfile.mkdtemp()] + install_folder = [tempfile.mkdtemp()] # Compute the module directory module_dir = os.path.join(*repo_path, module) @@ -529,18 +526,14 @@ def update(self, module=None): dry_run = not questionary.confirm( f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style ).unsafe_ask() - if not dry_run: - # The new module files are already installed. - # We just need to clear the directory and move the - # new files from the temporary directory - self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) - # Update modules.json with newly installed module if not dry_run: + # Clear the module directory and move the installed files there + self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) + # Update modules.json with newly installed module self.modules_json.update(modules_repo, module, version) - - # Don't save to a file, just iteratively update the variable else: + # Don't save to a file, just iteratively update the variable self.modules_json.update(modules_repo, module, version, write_file=False) if self.save_diff_fn: From 2686e9ae13e7ab6b813756be05b35d7e28e74257 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 13:49:30 +0200 Subject: [PATCH 78/93] Move mod json diff to method --- nf_core/modules/update.py | 49 +++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 6e7f7ecd19..5c1a54391c 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -346,6 +346,27 @@ def append_diff_file(self, module, diffs, module_dir, current_version, new_versi fh.write("*" * 60 + "\n") + def write_modules_json_diff(self, old_modules_json): + """ + Compare the new modules.json and builds a diff + + Args: + old_modules_json (nested dict): The old modules.json + """ + modules_json_diff = difflib.unified_diff( + json.dumps(old_modules_json, indent=4).splitlines(keepends=True), + json.dumps(self.modules_json.get_modules_json(), indent=4).splitlines(keepends=True), + fromfile=os.path.join(self.dir, "modules.json"), + tofile=os.path.join(self.dir, "modules.json"), + ) + + # Save diff for modules.json to file + with open(self.save_diff_fn, "a") as fh: + fh.write("Changes in './modules.json'\n") + for line in modules_json_diff: + fh.write(line) + fh.write("*" * 60 + "\n") + def print_diff(self, module, diffs, module_dir, current_version, new_version): """ Prints the diffs between two module versions @@ -406,6 +427,15 @@ def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_re log.debug(f"Updating module '{module}' to {new_version} from {modules_repo.fullname}") def update(self, module=None): + """ + Updates all modules or a specific module in a pipeline + + Args: + module (str): The module name to update + + Returns: + bool: True if the update was successful, False otherwise + """ self.module = module tool_config = nf_core.utils.load_tools_config(self.dir) @@ -537,29 +567,14 @@ def update(self, module=None): self.modules_json.update(modules_repo, module, version, write_file=False) if self.save_diff_fn: - # Compare the new modules.json and build a diff - modules_json_diff = difflib.unified_diff( - json.dumps(old_modules_json, indent=4).splitlines(keepends=True), - json.dumps(self.modules_json.get_modules_json(), indent=4).splitlines(keepends=True), - fromfile=os.path.join(self.dir, "modules.json"), - tofile=os.path.join(self.dir, "modules.json"), - ) - - # Save diff for modules.json to file - with open(self.save_diff_fn, "a") as fh: - fh.write("Changes in './modules.json'\n") - for line in modules_json_diff: - fh.write(line) - fh.write("*" * 60 + "\n") - + # Write the modules.json diff to the file + self.write_modules_json_diff(old_modules_json) log.info( f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " "can apply them by running the command :point_right:" f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) - else: - log.info("Updates complete :sparkles:") return exit_value From b8cfed319d40596d115e76707351a38a0d1b3edc Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 13:52:18 +0200 Subject: [PATCH 79/93] Move the update method --- nf_core/modules/update.py | 305 +++++++++++++++++++------------------- 1 file changed, 152 insertions(+), 153 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 5c1a54391c..214ec5078c 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -82,6 +82,158 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") + def update(self, module=None): + """ + Updates all modules or a specific module in a pipeline + + Args: + module (str): The module name to update + + Returns: + bool: True if the update was successful, False otherwise + """ + self.module = module + + tool_config = nf_core.utils.load_tools_config(self.dir) + self.update_config = tool_config.get("update", {}) + + self._parameter_checks() + + # Verify that 'modules.json' is consistent with the installed modules + self.modules_json = ModulesJson(self.dir) + self.modules_json.check_up_to_date() + + if not self.update_all and module is None: + choices = ["All modules", "Named module"] + self.update_all = ( + questionary.select( + "Update all modules or a single named module?", + choices=choices, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + == "All modules" + ) + + # Verify that the provided SHA exists in the repo + if self.sha and not self.modules_repo.sha_exists_on_branch(self.sha): + log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'") + return False + + self.get_pipeline_modules() + # Get the list of modules to update, and their version information + repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] + + # Save the current state of the modules.json + old_modules_json = self.modules_json.get_modules_json() + + # Ask if we should show the diffs (unless a filename was already given on the command line) + if not self.save_diff_fn and self.show_diff is None: + diff_type = questionary.select( + "Do you want to view diffs of the proposed changes?", + choices=[ + {"name": "No previews, just update everything", "value": 0}, + {"name": "Preview diff in terminal, choose whether to update files", "value": 1}, + {"name": "Just write diffs to a patch file", "value": 2}, + ], + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + + self.show_diff = diff_type == 1 + self.save_diff_fn = diff_type == 2 + + # Set up file to save diff + if self.save_diff_fn: # True or a string + self.setup_diff_file() + + exit_value = True + for modules_repo, module, sha in repos_mods_shas: + # Are we updating the files in place or not? + dry_run = self.show_diff or self.save_diff_fn + + # Check if the module we've been asked to update actually exists + if not modules_repo.module_exists(module): + warn_msg = f"Module '{module}' not found in remote '{modules_repo.fullname}' ({modules_repo.branch})" + if self.update_all: + warn_msg += ". Skipping..." + log.warning(warn_msg) + exit_value = False + continue + + current_version = self.modules_json.get_module_version(module, modules_repo.fullname) + + # Set the install folder based + repo_path = [self.dir, "modules"] + repo_path.extend(os.path.split(modules_repo.fullname)) + install_folder = [tempfile.mkdtemp()] + + # Compute the module directory + module_dir = os.path.join(*repo_path, module) + + if sha is not None: + version = sha + elif self.prompt: + version = nf_core.modules.module_utils.prompt_module_version_sha( + module, modules_repo=modules_repo, installed_sha=current_version + ) + else: + # Default to the latest commit for the module + version = modules_repo.get_latest_module_version(module) + + if current_version is not None and not self.force: + if current_version == version: + if self.sha or self.prompt: + log.info(f"'{modules_repo.fullname}/{module}' is already installed at {version}") + else: + log.info(f"'{modules_repo.fullname}/{module}' is already up to date") + continue + + if not dry_run: + log.info(f"Updating '{modules_repo.fullname}/{module}'") + log.debug(f"Updating module '{module}' to {version} from {modules_repo.remote_url}") + + log.debug(f"Removing old version of module '{module}'") + self.clear_module_dir(module, module_dir) + + # Download module files + if not self.install_module_files(module, version, modules_repo, install_folder): + exit_value = False + continue + + if dry_run: + # Compute the diffs for the module + diffs = self.get_module_diffs(install_folder, module, module_dir) + if self.save_diff_fn: + self.append_diff_file(module, diffs, module_dir, current_version, version) + elif self.show_diff: + self.print_diff(module, diffs, module_dir, current_version, version) + + # Ask the user if they want to install the module + dry_run = not questionary.confirm( + f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style + ).unsafe_ask() + + if not dry_run: + # Clear the module directory and move the installed files there + self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) + # Update modules.json with newly installed module + self.modules_json.update(modules_repo, module, version) + else: + # Don't save to a file, just iteratively update the variable + self.modules_json.update(modules_repo, module, version, write_file=False) + + if self.save_diff_fn: + # Write the modules.json diff to the file + self.write_modules_json_diff(old_modules_json) + log.info( + f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " + "can apply them by running the command :point_right:" + f" [bold magenta italic]git apply {self.save_diff_fn} [/]" + ) + else: + log.info("Updates complete :sparkles:") + + return exit_value + def get_single_module_info(self, module): """ Get modules repo and module verion info for a single module. @@ -425,156 +577,3 @@ def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_re shutil.move(path, os.path.join(module_dir, file)) log.info(f"Updating '{modules_repo.fullname}/{module}'") log.debug(f"Updating module '{module}' to {new_version} from {modules_repo.fullname}") - - def update(self, module=None): - """ - Updates all modules or a specific module in a pipeline - - Args: - module (str): The module name to update - - Returns: - bool: True if the update was successful, False otherwise - """ - self.module = module - - tool_config = nf_core.utils.load_tools_config(self.dir) - self.update_config = tool_config.get("update", {}) - - self._parameter_checks() - - # Verify that 'modules.json' is consistent with the installed modules - self.modules_json = ModulesJson(self.dir) - self.modules_json.check_up_to_date() - - if not self.update_all and module is None: - choices = ["All modules", "Named module"] - self.update_all = ( - questionary.select( - "Update all modules or a single named module?", - choices=choices, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - == "All modules" - ) - - # Verify that the provided SHA exists in the repo - if self.sha and not self.modules_repo.sha_exists_on_branch(self.sha): - log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'") - return False - - self.get_pipeline_modules() - # Get the list of modules to update, and their version information - repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] - - # Save the current state of the modules.json - old_modules_json = self.modules_json.get_modules_json() - - # Ask if we should show the diffs (unless a filename was already given on the command line) - if not self.save_diff_fn and self.show_diff is None: - diff_type = questionary.select( - "Do you want to view diffs of the proposed changes?", - choices=[ - {"name": "No previews, just update everything", "value": 0}, - {"name": "Preview diff in terminal, choose whether to update files", "value": 1}, - {"name": "Just write diffs to a patch file", "value": 2}, - ], - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - - self.show_diff = diff_type == 1 - self.save_diff_fn = diff_type == 2 - - # Set up file to save diff - if self.save_diff_fn: # True or a string - self.setup_diff_file() - - exit_value = True - for modules_repo, module, sha in repos_mods_shas: - - # Are we updating the files in place or not? - dry_run = self.show_diff or self.save_diff_fn - - # Check if the module we've been asked to update actually exists - if not modules_repo.module_exists(module): - warn_msg = f"Module '{module}' not found in remote '{modules_repo.fullname}' ({modules_repo.branch})" - if self.update_all: - warn_msg += ". Skipping..." - log.warning(warn_msg) - exit_value = False - continue - - current_version = self.modules_json.get_module_version(module, modules_repo.fullname) - - # Set the install folder based - repo_path = [self.dir, "modules"] - repo_path.extend(os.path.split(modules_repo.fullname)) - install_folder = [tempfile.mkdtemp()] - - # Compute the module directory - module_dir = os.path.join(*repo_path, module) - - if sha is not None: - version = sha - elif self.prompt: - version = nf_core.modules.module_utils.prompt_module_version_sha( - module, modules_repo=modules_repo, installed_sha=current_version - ) - else: - # Default to the latest commit for the module - version = modules_repo.get_latest_module_version(module) - - if current_version is not None and not self.force: - if current_version == version: - if self.sha or self.prompt: - log.info(f"'{modules_repo.fullname}/{module}' is already installed at {version}") - else: - log.info(f"'{modules_repo.fullname}/{module}' is already up to date") - continue - - if not dry_run: - log.info(f"Updating '{modules_repo.fullname}/{module}'") - log.debug(f"Updating module '{module}' to {version} from {modules_repo.remote_url}") - - log.debug(f"Removing old version of module '{module}'") - self.clear_module_dir(module, module_dir) - - # Download module files - if not self.install_module_files(module, version, modules_repo, install_folder): - exit_value = False - continue - - if dry_run: - # Compute the diffs for the module - diffs = self.get_module_diffs(install_folder, module, module_dir) - if self.save_diff_fn: - self.append_diff_file(module, diffs, module_dir, current_version, version) - elif self.show_diff: - self.print_diff(module, diffs, module_dir, current_version, version) - - # Ask the user if they want to install the module - dry_run = not questionary.confirm( - f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style - ).unsafe_ask() - - if not dry_run: - # Clear the module directory and move the installed files there - self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) - # Update modules.json with newly installed module - self.modules_json.update(modules_repo, module, version) - else: - # Don't save to a file, just iteratively update the variable - self.modules_json.update(modules_repo, module, version, write_file=False) - - if self.save_diff_fn: - # Write the modules.json diff to the file - self.write_modules_json_diff(old_modules_json) - log.info( - f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " - "can apply them by running the command :point_right:" - f" [bold magenta italic]git apply {self.save_diff_fn} [/]" - ) - else: - log.info("Updates complete :sparkles:") - - return exit_value From 100d311fe0b2cbdf8312659a8ee0f978e237dcd1 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Wed, 20 Jul 2022 13:57:26 +0200 Subject: [PATCH 80/93] fix variable name --- nf_core/modules/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 6e7f7ecd19..0ddbcbafd3 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -45,7 +45,7 @@ def __init__( self.save_diff_fn = save_diff_fn self.module = None self.update_config = None - self.module_json = None + self.modules_json = None class DiffEnum(enum.Enum): """ From 4522d69c0aa9f46402b926a28b4ae6572e2dd770 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 14:13:20 +0200 Subject: [PATCH 81/93] More misc. refactoring --- nf_core/modules/update.py | 64 ++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 214ec5078c..f38469427b 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -47,6 +47,9 @@ def __init__( self.update_config = None self.module_json = None + # Fetch the list of pipeline modules + self.get_pipeline_modules() + class DiffEnum(enum.Enum): """ Enumeration for keeping track of @@ -115,11 +118,10 @@ def update(self, module=None): ) # Verify that the provided SHA exists in the repo - if self.sha and not self.modules_repo.sha_exists_on_branch(self.sha): + if self.sha is not None and not self.modules_repo.sha_exists_on_branch(self.sha): log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'") return False - self.get_pipeline_modules() # Get the list of modules to update, and their version information repos_mods_shas = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] @@ -141,33 +143,23 @@ def update(self, module=None): self.show_diff = diff_type == 1 self.save_diff_fn = diff_type == 2 - # Set up file to save diff if self.save_diff_fn: # True or a string self.setup_diff_file() + # Loop through all modules to be updated + # and do the requested action on them exit_value = True for modules_repo, module, sha in repos_mods_shas: # Are we updating the files in place or not? dry_run = self.show_diff or self.save_diff_fn - # Check if the module we've been asked to update actually exists - if not modules_repo.module_exists(module): - warn_msg = f"Module '{module}' not found in remote '{modules_repo.fullname}' ({modules_repo.branch})" - if self.update_all: - warn_msg += ". Skipping..." - log.warning(warn_msg) - exit_value = False - continue - current_version = self.modules_json.get_module_version(module, modules_repo.fullname) - # Set the install folder based - repo_path = [self.dir, "modules"] - repo_path.extend(os.path.split(modules_repo.fullname)) + # Set the temporary installation folder install_folder = [tempfile.mkdtemp()] # Compute the module directory - module_dir = os.path.join(*repo_path, module) + module_dir = os.path.join(self.dir, "modules", os.path.split(modules_repo.fullname), module) if sha is not None: version = sha @@ -187,13 +179,6 @@ def update(self, module=None): log.info(f"'{modules_repo.fullname}/{module}' is already up to date") continue - if not dry_run: - log.info(f"Updating '{modules_repo.fullname}/{module}'") - log.debug(f"Updating module '{module}' to {version} from {modules_repo.remote_url}") - - log.debug(f"Removing old version of module '{module}'") - self.clear_module_dir(module, module_dir) - # Download module files if not self.install_module_files(module, version, modules_repo, install_folder): exit_value = False @@ -311,23 +296,23 @@ def get_all_modules_info(self): skipped_modules = [] overridden_repos = [] overridden_modules = [] - repos_mods_shas = {} + 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(): if repo_name not in self.update_config or self.update_config[repo_name] is True: - repos_mods_shas[repo_name] = [(module, self.sha) for module in modules] + modules_info[repo_name] = [(module, self.sha) for module in modules] elif isinstance(self.update_config[repo_name], dict): # If it is a dict, then there are entries for individual modules repo_config = self.update_config[repo_name] - repos_mods_shas[repo_name] = [] + modules_info[repo_name] = [] for module in modules: if module not in repo_config or repo_config[module] is True: - repos_mods_shas[repo_name].append((module, self.sha)) + modules_info[repo_name].append((module, self.sha)) elif isinstance(repo_config[module], str): # If a string is given it is the commit SHA to which we should update to custom_sha = repo_config[module] - repos_mods_shas[repo_name].append((module, custom_sha)) + modules_info[repo_name].append((module, custom_sha)) if self.sha is not None: overridden_modules.append(module) elif repo_config[module] is False: @@ -338,7 +323,7 @@ def get_all_modules_info(self): elif isinstance(self.update_config[repo_name], str): # If a string is given it is the commit SHA to which we should update to custom_sha = self.update_config[repo_name] - repos_mods_shas[repo_name] = [(module_name, custom_sha) for module_name in modules] + modules_info[repo_name] = [(module_name, custom_sha) for module_name in modules] if self.sha is not None: overridden_repos.append(repo_name) elif self.update_config[repo_name] is False: @@ -368,19 +353,29 @@ def get_all_modules_info(self): ) # Get the git urls from the modules.json - repos_mods_shas = [ + modules_info = [ (self.modules_json.get_git_url(repo_name), self.modules_json.get_base_path(repo_name), mods_shas) - for repo_name, mods_shas in repos_mods_shas.items() + for repo_name, mods_shas in modules_info.items() ] # Create ModulesRepo objects - repos_mods_shas = [ + modules_info = [ (ModulesRepo(remote_url=repo_url, base_path=base_path), mods_shas) - for repo_url, base_path, mods_shas in repos_mods_shas + for repo_url, base_path, mods_shas in modules_info ] # Flatten and return the list - return [(repo, mod, sha) for repo, mods_shas in repos_mods_shas for mod, sha in mods_shas] + modules_info = [(repo, mod, sha) for repo, mods_shas in modules_info for mod, sha in mods_shas] + + # Verify that that all modules exist in their respective ModulesRepo, remove those that don't + i = 0 + while i < len(modules_info): + repo, module, sha = modules_info[i] + if repo.module_exists(module): + i += 1 + else: + log.warning(f"Module '{module}' does not exist in '{repo.fullname}'. Skipping...") + modules_info.pop(i) def setup_diff_file(self): if self.save_diff_fn is True: @@ -569,6 +564,7 @@ def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_re """ temp_module_dir = os.path.join(*install_folder, module) files = os.listdir(temp_module_dir) + log.debug(f"Removing old version of module '{module}'") self.clear_module_dir(module, module_dir) os.makedirs(module_dir) for file in files: From 7df4f5707f690a4327c228ee19fa87d8a8e40322 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 14:20:38 +0200 Subject: [PATCH 82/93] Fix tests --- nf_core/modules/update.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index d9e0d5692d..0b257f233c 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -159,7 +159,7 @@ def update(self, module=None): install_folder = [tempfile.mkdtemp()] # Compute the module directory - module_dir = os.path.join(self.dir, "modules", os.path.split(modules_repo.fullname), module) + module_dir = os.path.join(self.dir, "modules", modules_repo.fullname, module) if sha is not None: version = sha @@ -377,6 +377,8 @@ def get_all_modules_info(self): log.warning(f"Module '{module}' does not exist in '{repo.fullname}'. Skipping...") modules_info.pop(i) + return modules_info + def setup_diff_file(self): if self.save_diff_fn is True: # From questionary - no filename yet From e4f6e9be1a52ecad0d709eb128c0f1ec0bbc75e8 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 15:42:49 +0200 Subject: [PATCH 83/93] Don't pass install folder as a list --- nf_core/modules/install.py | 7 +++---- nf_core/modules/modules_repo.py | 2 +- nf_core/modules/update.py | 16 ++++++++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/nf_core/modules/install.py b/nf_core/modules/install.py index 0ea3c827dc..e6322f3ff2 100644 --- a/nf_core/modules/install.py +++ b/nf_core/modules/install.py @@ -85,11 +85,10 @@ def install(self, module): current_version = modules_json.get_module_version(module, self.modules_repo.fullname) # Set the install folder based on the repository name - install_folder = [self.dir, "modules"] - install_folder.extend(os.path.split(self.modules_repo.fullname)) + install_folder = os.path.join(self.dir, "modules", self.modules_repo.fullname) # Compute the module directory - module_dir = os.path.join(*install_folder, module) + module_dir = os.path.join(install_folder, module) # Check that the module is not already installed if (current_version is not None and os.path.exists(module_dir)) and not self.force: @@ -135,7 +134,7 @@ def install(self, module): # Print include statement module_name = "_".join(module.upper().split("/")) - log.info(f"Include statement: include {{ {module_name} }} from '.{os.path.join(*install_folder, module)}/main'") + log.info(f"Include statement: include {{ {module_name} }} from '.{os.path.join(install_folder, module)}/main'") # Update module.json with newly installed module modules_json.update(self.modules_repo, module, version) diff --git a/nf_core/modules/modules_repo.py b/nf_core/modules/modules_repo.py index 9f7783c3a0..94e5b2bc61 100644 --- a/nf_core/modules/modules_repo.py +++ b/nf_core/modules/modules_repo.py @@ -284,7 +284,7 @@ def install_module(self, module_name, install_dir, commit): return False # Copy the files from the repo to the install folder - shutil.copytree(self.get_module_dir(module_name), os.path.join(*install_dir, module_name)) + shutil.copytree(self.get_module_dir(module_name), os.path.join(install_dir, module_name)) # Switch back to the tip of the branch self.checkout_branch() diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 0b257f233c..0b1894d275 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -156,7 +156,7 @@ def update(self, module=None): current_version = self.modules_json.get_module_version(module, modules_repo.fullname) # Set the temporary installation folder - install_folder = [tempfile.mkdtemp()] + install_folder = tempfile.mkdtemp() # Compute the module directory module_dir = os.path.join(self.dir, "modules", modules_repo.fullname, module) @@ -168,7 +168,6 @@ def update(self, module=None): module, modules_repo=modules_repo, installed_sha=current_version ) else: - # Default to the latest commit for the module version = modules_repo.get_latest_module_version(module) if current_version is not None and not self.force: @@ -367,7 +366,8 @@ def get_all_modules_info(self): # Flatten and return the list modules_info = [(repo, mod, sha) for repo, mods_shas in modules_info for mod, sha in mods_shas] - # Verify that that all modules exist in their respective ModulesRepo, remove those that don't + # Verify that that all modules exist in their respective ModulesRepo, + # don't try to update those that don't i = 0 while i < len(modules_info): repo, module, sha = modules_info[i] @@ -380,6 +380,10 @@ def get_all_modules_info(self): return modules_info def setup_diff_file(self): + """ + Prompts for a file name if the the save diff option was choosen interactively. + Then creates the file for saving the diff. + """ if self.save_diff_fn is True: # From questionary - no filename yet self.save_diff_fn = questionary.text( @@ -413,11 +417,11 @@ def get_module_diffs(self, install_folder, module, module_dir): diffs = {} # Get all unique filenames in the two folders. # `dict.fromkeys()` is used instead of `set()` to preserve order - files = dict.fromkeys(os.listdir(os.path.join(*install_folder, module))) + files = dict.fromkeys(os.listdir(os.path.join(install_folder, module))) files.update(dict.fromkeys(os.listdir(module_dir))) files = list(files) - temp_folder = os.path.join(*install_folder, module) + temp_folder = os.path.join(install_folder, module) # Loop through all the module files and compute their diffs if needed for file in files: @@ -564,7 +568,7 @@ def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_re was installed new_version (str): The version of the module that was installed """ - temp_module_dir = os.path.join(*install_folder, module) + temp_module_dir = os.path.join(install_folder, module) files = os.listdir(temp_module_dir) log.debug(f"Removing old version of module '{module}'") self.clear_module_dir(module, module_dir) From 1949497c9859b73b56d8242287eec86b8ad44431 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 15:58:23 +0200 Subject: [PATCH 84/93] Fix install dir issue in ModulesJson --- nf_core/modules/modules_json.py | 5 ++--- nf_core/modules/update.py | 11 +++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index cd9b09c23d..2a1313c50a 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -301,7 +301,7 @@ def check_up_to_date(self): base_path = contents["base_path"] modules_repo = nf_core.modules.modules_repo.ModulesRepo(remote_url=remote, base_path=base_path) - install_folder = os.path.split(modules_repo.fullname) + install_dir = os.path.join(self.dir, "modules", modules_repo.fullname) for module, entry in modules.items(): sha = entry.get("git_sha") @@ -313,8 +313,7 @@ def check_up_to_date(self): ) remove_from_mod_json[repo].append(module) continue - module_dir = [self.dir, "modules", *install_folder] - if not modules_repo.install_module(module, module_dir, sha): + if not modules_repo.install_module(module, install_dir, sha): if repo not in remove_from_mod_json: remove_from_mod_json[repo] = [] log.warning(f"Could not install module '{module}' in '{repo}' - removing from modules.json") diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 0b1894d275..3eeb8c4348 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -198,7 +198,7 @@ def update(self, module=None): if not dry_run: # Clear the module directory and move the installed files there - self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo, version) + self.move_files_from_tmp_dir(module, module_dir, install_folder, modules_repo.fullname, version) # Update modules.json with newly installed module self.modules_json.update(modules_repo, module, version) else: @@ -555,7 +555,7 @@ def print_diff(self, module, diffs, module_dir, current_version, new_version): # Pretty print the diff using the pygments diff lexer console.print(Syntax("".join(diff), "diff", theme="ansi_light")) - def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_repo, new_version): + def move_files_from_tmp_dir(self, module, module_dir, install_folder, repo_name, new_version): """ Move the files from the temporary installation directory to the module directory. @@ -570,12 +570,15 @@ def move_files_from_tmp_dir(self, module, module_dir, install_folder, modules_re """ temp_module_dir = os.path.join(install_folder, module) files = os.listdir(temp_module_dir) + log.debug(f"Removing old version of module '{module}'") self.clear_module_dir(module, module_dir) + os.makedirs(module_dir) for file in files: path = os.path.join(temp_module_dir, file) if os.path.exists(path): shutil.move(path, os.path.join(module_dir, file)) - log.info(f"Updating '{modules_repo.fullname}/{module}'") - log.debug(f"Updating module '{module}' to {new_version} from {modules_repo.fullname}") + + log.info(f"Updating '{repo_name}/{module}'") + log.debug(f"Updating module '{module}' to {new_version} from {repo_name}") From 9d2c29cc213a0d7bd4102cd303905e4254094add Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Wed, 20 Jul 2022 16:09:04 +0200 Subject: [PATCH 85/93] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a74672fe37..66b28e4b52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ - Command `nf-core modules test` obtains module name suggestions from installed modules ([#1624](https://github.com/nf-core/tools/pull/1624)) - Add `--base-path` flag to `nf-core modules` to specify the base path for the modules in a remote. Also refactored `modules.json` code. ([#1643](https://github.com/nf-core/tools/issues/1643)) - Rename methods in `ModulesJson` to remove explicit reference to `modules.json` +- Refactor `nf-core modules install` ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16] From 9145b6ccd9059be7c3c39c4c659f5e3950fa570a Mon Sep 17 00:00:00 2001 From: Erik Danielsson <53212377+ErikDanielsson@users.noreply.github.com> Date: Thu, 21 Jul 2022 09:16:39 +0200 Subject: [PATCH 86/93] Update CHANGELOG.md Co-authored-by: Fabian Egli --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b28e4b52..6a676a27a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ - Command `nf-core modules test` obtains module name suggestions from installed modules ([#1624](https://github.com/nf-core/tools/pull/1624)) - Add `--base-path` flag to `nf-core modules` to specify the base path for the modules in a remote. Also refactored `modules.json` code. ([#1643](https://github.com/nf-core/tools/issues/1643)) - Rename methods in `ModulesJson` to remove explicit reference to `modules.json` -- Refactor `nf-core modules install` +- The `--diff` flag of `nf-core modules update` works now as expected ([#1536](https://github.com/nf-core/tools/pull/1536)) ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16] From 9cdc79c313e7705bccf155a912eed3fa9e02d9df Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Thu, 21 Jul 2022 09:21:36 +0200 Subject: [PATCH 87/93] Fix short flag of '--show-diff' and update CHANGELOG --- CHANGELOG.md | 2 +- nf_core/__main__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a676a27a1..2f90bbf49c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ - Command `nf-core modules test` obtains module name suggestions from installed modules ([#1624](https://github.com/nf-core/tools/pull/1624)) - Add `--base-path` flag to `nf-core modules` to specify the base path for the modules in a remote. Also refactored `modules.json` code. ([#1643](https://github.com/nf-core/tools/issues/1643)) - Rename methods in `ModulesJson` to remove explicit reference to `modules.json` -- The `--diff` flag of `nf-core modules update` works now as expected ([#1536](https://github.com/nf-core/tools/pull/1536)) +- Fix inconsistencies in the `--show-diff` flag `nf-core modules update`. Refactor `nf-core modules update` ([#1536](https://github.com/nf-core/tools/pull/1536)) ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16] diff --git a/nf_core/__main__.py b/nf_core/__main__.py index e66d9cead2..e859929de1 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -504,7 +504,7 @@ def install(ctx, tool, dir, prompt, force, sha): help="Preview / no preview of changes before applying", ) @click.option( - "-p", + "-D", "--save-diff", type=str, metavar="", From be5cf77ed635b693224158545ed34e358eae5145 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 21 Jul 2022 09:37:03 +0200 Subject: [PATCH 88/93] some docstring improvements --- nf_core/modules/update.py | 88 +++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 3eeb8c4348..85209c0eca 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -51,9 +51,7 @@ def __init__( self.get_pipeline_modules() class DiffEnum(enum.Enum): - """ - Enumeration for keeping track of - the diff status of a pair of files + """Enumeration to keeping track of file diffs. Used for the --save-diff and --preview options """ @@ -64,10 +62,10 @@ class DiffEnum(enum.Enum): REMOVED = enum.auto() def _parameter_checks(self): - """Check the compatibilty of the supplied parameters. + """Checks the compatibilty of the supplied parameters. - Checks: - - Either `--preview` or `--save_diff` can be specified, not both. + Raises: + UserWarning: if any checks fail. """ if self.save_diff_fn and self.show_diff: @@ -86,14 +84,13 @@ def _parameter_checks(self): raise UserWarning("The command was not run in a valid pipeline directory.") def update(self, module=None): - """ - Updates all modules or a specific module in a pipeline + """Updates a specified module or all modules modules in a pipeline. Args: - module (str): The module name to update + module (str): The name of the module to update. Returns: - bool: True if the update was successful, False otherwise + bool: True if the update was successful, False otherwise. """ self.module = module @@ -219,8 +216,7 @@ def update(self, module=None): return exit_value def get_single_module_info(self, module): - """ - Get modules repo and module verion info for a single module. + """Collects the module repository, version and sha for a module. Information about the module version in the '.nf-core.yml' overrides the '--sha' option @@ -281,11 +277,9 @@ def get_single_module_info(self, module): return (self.modules_repo, module, sha) def get_all_modules_info(self): - """ - Get modules repo and module version information for all modules. + """Collects the module repository, version and sha for all modules. - Information about the module version in the '.nf-core.yml' overrides - the '--sha' option + Information about the module version in the '.nf-core.yml' overrides the '--sha' option. Returns: [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object, @@ -380,8 +374,10 @@ def get_all_modules_info(self): return modules_info def setup_diff_file(self): - """ - Prompts for a file name if the the save diff option was choosen interactively. + """Sets up the diff file. + + If the save diff option was choosen interactively, the user is asked to supply a name for the diff file. + Then creates the file for saving the diff. """ if self.save_diff_fn is True: @@ -400,18 +396,16 @@ def setup_diff_file(self): ).unsafe_ask() def get_module_diffs(self, install_folder, module, module_dir): - """ - Compute the diff between the current module version - and the new version. + """Computes the diff between the current and the new module version. Args: - install_folder ([str]): The folder where the module is installed - module (str): The module name - module_dir (str): The directory containing the current version of the module + install_folder ([str]): The folder where the module is installed. + module (str): The module name. + module_dir (str): The directory containing the current version of the module. Returns: - dict[str, (ModuleUpdate.DiffEnum, str)]: A dictionary containing - the diff type and the diff string (empty if no diff) + dict[str, (ModuleUpdate.DiffEnum, str)]: A dictionary containing. + the diff type and the diff string (empty if no diff). """ diffs = {} @@ -457,16 +451,15 @@ def get_module_diffs(self, install_folder, module, module_dir): return diffs def append_diff_file(self, module, diffs, module_dir, current_version, new_version): - """ - Writes the diffs of a module to the diff file. + """Writes the diffs of a module to the diff file. Args: - module (str): The module name + module (str): The module name. diffs (dict[str, (ModuleUpdate.DiffEnum, str)]): A dictionary containing - the type of change and the diff (if any) - module_dir (str): The path to the current installation of the module - current_version (str): The installed version of the module - new_version (str): The version of the module the diff is computed against + the type of change and the diff (if any). + module_dir (str): The path to the current installation of the module. + current_version (str): The installed version of the module. + new_version (str): The version of the module the diff is computed against. """ log.info(f"Writing diff of '{module}' to '{self.save_diff_fn}'") with open(self.save_diff_fn, "a") as fh: @@ -500,8 +493,7 @@ def append_diff_file(self, module, diffs, module_dir, current_version, new_versi fh.write("*" * 60 + "\n") def write_modules_json_diff(self, old_modules_json): - """ - Compare the new modules.json and builds a diff + """Creates a diff between the old to the new modules.json and writes it to the diff file. Args: old_modules_json (nested dict): The old modules.json @@ -521,16 +513,15 @@ def write_modules_json_diff(self, old_modules_json): fh.write("*" * 60 + "\n") def print_diff(self, module, diffs, module_dir, current_version, new_version): - """ - Prints the diffs between two module versions + """Prints the diffs between two module versions. Args: module (str): The module name diffs (dict[str, (ModuleUpdate.DiffEnum, str)]): A dictionary containing - the type of change and the diff (if any) - module_dir (str): The path to the current installation of the module - current_version (str): The installed version of the module - new_version (str): The version of the module the diff is computed against + the type of change and the diff (if any). + module_dir (str): The path to the current installation of the module. + current_version (str): The installed version of the module. + new_version (str): The version of the module the diff is computed against. """ console = Console(force_terminal=nf_core.utils.rich_force_colors()) log.info( @@ -556,17 +547,14 @@ def print_diff(self, module, diffs, module_dir, current_version, new_version): console.print(Syntax("".join(diff), "diff", theme="ansi_light")) def move_files_from_tmp_dir(self, module, module_dir, install_folder, repo_name, new_version): - """ - Move the files from the temporary installation directory to the - module directory. + """Move the files from the temporary to the installation directory. Args: - module (str): The module name - module_dir (str): The path to the module directory - install_folder [str]: The path to the temporary installation directory - modules_repo (ModulesRepo): The ModulesRepo object from which the module - was installed - new_version (str): The version of the module that was installed + module (str): The module name. + module_dir (str): The path to the module directory. + install_folder [str]: The path to the temporary installation directory. + modules_repo (ModulesRepo): The ModulesRepo object from which the module was installed. + new_version (str): The version of the module that was installed. """ temp_module_dir = os.path.join(install_folder, module) files = os.listdir(temp_module_dir) From f39d97d5d5b8e4743dea3312305a596a90bd372c Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 21 Jul 2022 09:49:47 +0200 Subject: [PATCH 89/93] use questionary.path for file name input --- nf_core/modules/update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 85209c0eca..dfe212514d 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -382,7 +382,7 @@ def setup_diff_file(self): """ if self.save_diff_fn is True: # From questionary - no filename yet - self.save_diff_fn = questionary.text( + self.save_diff_fn = questionary.path( "Enter the filename: ", style=nf_core.utils.nfcore_question_style ).unsafe_ask() # Check if filename already exists (questionary or cli) @@ -390,7 +390,7 @@ def setup_diff_file(self): if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): os.remove(self.save_diff_fn) break - self.save_diff_fn = questionary.text( + self.save_diff_fn = questionary.path( "Enter a new filename: ", style=nf_core.utils.nfcore_question_style, ).unsafe_ask() From 8fddf53a92be836132cc5f55549b89435453c46f Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 21 Jul 2022 09:50:40 +0200 Subject: [PATCH 90/93] break too long line --- nf_core/modules/update.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index dfe212514d..b9c4b1da14 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -342,7 +342,8 @@ def get_all_modules_info(self): if overridden_modules: overridden_str = "', '".join(overridden_modules) log.info( - f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with '.nf-core.yml' entry: '{overridden_str}'" + f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with " + f"'.nf-core.yml' entry: '{overridden_str}'" ) # Get the git urls from the modules.json From d949be1c1560f3ae3ab6d5b5a0fa208515d1fbf0 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 21 Jul 2022 09:51:36 +0200 Subject: [PATCH 91/93] ensure diff file exists after diff file setup --- nf_core/modules/update.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index b9c4b1da14..ff2489c119 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -5,6 +5,7 @@ import os import shutil import tempfile +from pathlib import Path import questionary from rich.console import Console @@ -386,8 +387,11 @@ def setup_diff_file(self): self.save_diff_fn = questionary.path( "Enter the filename: ", style=nf_core.utils.nfcore_question_style ).unsafe_ask() + + self.save_diff_fn = Path(self.save_diff_fn) + # Check if filename already exists (questionary or cli) - while os.path.exists(self.save_diff_fn): + while self.save_diff_fn.exists(): if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): os.remove(self.save_diff_fn) break @@ -395,6 +399,10 @@ def setup_diff_file(self): "Enter a new filename: ", style=nf_core.utils.nfcore_question_style, ).unsafe_ask() + self.save_diff_fn = Path(self.save_diff_fn) + + # This guarantees that the file exists after calling the function + self.save_diff_fn.touch() def get_module_diffs(self, install_folder, module, module_dir): """Computes the diff between the current and the new module version. From 88ff09f16673aa09930d07b13a3a3e2e2053bc94 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Thu, 21 Jul 2022 10:42:42 +0200 Subject: [PATCH 92/93] Fix changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f90bbf49c..4f38307c0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ - Command `nf-core modules test` obtains module name suggestions from installed modules ([#1624](https://github.com/nf-core/tools/pull/1624)) - Add `--base-path` flag to `nf-core modules` to specify the base path for the modules in a remote. Also refactored `modules.json` code. ([#1643](https://github.com/nf-core/tools/issues/1643)) - Rename methods in `ModulesJson` to remove explicit reference to `modules.json` -- Fix inconsistencies in the `--show-diff` flag `nf-core modules update`. Refactor `nf-core modules update` ([#1536](https://github.com/nf-core/tools/pull/1536)) +- Fix inconsistencies in the `--save-diff` flag `nf-core modules update`. Refactor `nf-core modules update` ([#1536](https://github.com/nf-core/tools/pull/1536)) ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16] From ee593923cdea80e7ab48380f22150c9a6069f685 Mon Sep 17 00:00:00 2001 From: Erik Danielsson Date: Thu, 21 Jul 2022 12:09:54 +0200 Subject: [PATCH 93/93] Update docs --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0be79c90ab..6f39b738f9 100644 --- a/README.md +++ b/README.md @@ -1119,8 +1119,8 @@ There are five additional flags that you can use with this command: - `--force`: Reinstall module even if it appears to be up to date - `--prompt`: Select the module version using a cli prompt. - `--sha `: Install the module at a specific commit from the `nf-core/modules` repository. -- `--diff`: Show the diff between the installed files and the new version before installing. -- `--diff-file `: Specify where the diffs between the local and remote versions of a module should be written +- `--preview/--no-preview`: Show the diff between the installed files and the new version before installing. +- `--save-diff `: Save diffs to a file instead of updating in place. The diffs can then be applied with `git apply `. - `--all`: Use this flag to run the command on all modules in the pipeline. If you don't want to update certain modules or want to update them to specific versions, you can make use of the `.nf-core.yml` configuration file. For example, you can prevent the `star/align` module installed from `nf-core/modules` from being updated by adding the following to the `.nf-core.yml` file: