From 1d04268c972873c058ca65ede5310f6be5f309c8 Mon Sep 17 00:00:00 2001 From: mashehu Date: Fri, 16 Feb 2024 17:09:24 +0100 Subject: [PATCH 1/4] add prompt for extra files --- nf_core/components/update.py | 37 ++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 077cb2b840..545fd9ebad 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -5,6 +5,7 @@ from pathlib import Path import questionary +import rich.prompt import nf_core.modules.modules_utils import nf_core.utils @@ -76,7 +77,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, component=None, silent=False, updated=None, check_diff_exist=True): + def update(self, component=None, silent=False, updated=None, check_diff_exist=True) -> bool: """Updates a specified module/subworkflow or all modules/subworkflows in a pipeline. If updating a subworkflow: updates all modules used in that subworkflow. @@ -188,7 +189,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr continue # Download component files - if not self.install_component_files(component, version, modules_repo, install_tmp_dir): + if not self.install_component_files(component, version, modules_repo, str(install_tmp_dir)): exit_value = False continue @@ -282,7 +283,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr if not dry_run: # Clear the component directory and move the installed files there - self.move_files_from_tmp_dir(component, install_tmp_dir, modules_repo.repo_path, version) + self.move_files_from_tmp_dir(component, str(install_tmp_dir), modules_repo.repo_path, version) # Update modules.json with newly installed component self.modules_json.update(self.component_type, modules_repo, component, version, installed_by=None) updated.append(component) @@ -727,7 +728,7 @@ def setup_diff_file(self, check_diff_exist=True): # This guarantees that the file exists after calling the function self.save_diff_fn.touch() - def move_files_from_tmp_dir(self, component, install_folder, repo_path, new_version): + def move_files_from_tmp_dir(self, component: str, install_folder: str, repo_path: str, new_version: str) -> None: """Move the files from the temporary to the installation directory. Args: @@ -736,18 +737,34 @@ def move_files_from_tmp_dir(self, component, install_folder, repo_path, new_vers repo_path (str): The name of the directory where modules/subworkflows are installed new_version (str): The version of the module/subworkflow that was installed. """ - temp_component_dir = os.path.join(install_folder, component) - files = os.listdir(temp_component_dir) - pipeline_path = os.path.join(self.dir, self.component_type, repo_path, component) + temp_component_dir = Path(install_folder, component) + files = [f.name for f in temp_component_dir.iterdir() if f.is_file()] + pipeline_path = Path(self.dir, self.component_type, repo_path, component) log.debug(f"Removing old version of {self.component_type[:-1]} '{component}'") - self.clear_component_dir(component, pipeline_path) + # check if both directories have the same file and warn if the compnent_dir has more files + if pipeline_path.exists(): + pipeline_files = [f.name for f in pipeline_path.iterdir() if f.is_file()] + if pipeline_files: + # ask if the user wants to move the file from the current directory to the new one + for file in pipeline_files: + if file not in files: + # use prompt to ask the user if they want to move the file + if rich.prompt.Confirm().ask( + f"File '{file}' exists in the current {self.component_type[:-1]} '{component}' directory. Do you want to keep it in the updated version?" + ): + shutil.move(Path(pipeline_path, file), Path(temp_component_dir, file)) + files.append(file) + + else: + log.debug(f"Creating new {self.component_type[:-1]} '{component}' in '{self.component_type}/{repo_path}'") + self.clear_component_dir(component, str(pipeline_path)) os.makedirs(pipeline_path) for file in files: - path = os.path.join(temp_component_dir, file) + path = Path(temp_component_dir, file) if os.path.exists(path): - shutil.move(path, os.path.join(pipeline_path, file)) + shutil.move(path, Path(pipeline_path, file)) log.info(f"Updating '{repo_path}/{component}'") log.debug(f"Updating {self.component_type[:-1]} '{component}' to {new_version} from {repo_path}") From 31dc4376948cf6cd7c7c6b4d34ae226b8788655b Mon Sep 17 00:00:00 2001 From: mashehu Date: Fri, 16 Feb 2024 22:20:52 +0100 Subject: [PATCH 2/4] simplify logic and add tests --- nf_core/components/update.py | 18 ++++++------------ tests/modules/update.py | 18 ++++++++++++++++++ tests/test_modules.py | 1 + 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 545fd9ebad..841ed45659 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -5,7 +5,6 @@ from pathlib import Path import questionary -import rich.prompt import nf_core.modules.modules_utils import nf_core.utils @@ -741,23 +740,18 @@ def move_files_from_tmp_dir(self, component: str, install_folder: str, repo_path files = [f.name for f in temp_component_dir.iterdir() if f.is_file()] pipeline_path = Path(self.dir, self.component_type, repo_path, component) - log.debug(f"Removing old version of {self.component_type[:-1]} '{component}'") # check if both directories have the same file and warn if the compnent_dir has more files if pipeline_path.exists(): pipeline_files = [f.name for f in pipeline_path.iterdir() if f.is_file()] - if pipeline_files: - # ask if the user wants to move the file from the current directory to the new one - for file in pipeline_files: - if file not in files: - # use prompt to ask the user if they want to move the file - if rich.prompt.Confirm().ask( - f"File '{file}' exists in the current {self.component_type[:-1]} '{component}' directory. Do you want to keep it in the updated version?" - ): - shutil.move(Path(pipeline_path, file), Path(temp_component_dir, file)) - files.append(file) + if "nextflow.config" in pipeline_files: + log.debug(f"Moving '{component}/nextflow.config' to updated component") + shutil.move(Path(pipeline_path, "nextflow.config"), Path(temp_component_dir, "nextflow.config")) + files.append("nextflow.config") else: log.debug(f"Creating new {self.component_type[:-1]} '{component}' in '{self.component_type}/{repo_path}'") + + log.debug(f"Removing old version of {self.component_type[:-1]} '{component}'") self.clear_component_dir(component, str(pipeline_path)) os.makedirs(pipeline_path) diff --git a/tests/modules/update.py b/tests/modules/update.py index 5cf24b56f0..80cd5c2eed 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -376,3 +376,21 @@ def cmp_module(dir1, dir2): """Compare two versions of the same module""" files = ["main.nf", "meta.yml"] return all(filecmp.cmp(os.path.join(dir1, f), os.path.join(dir2, f), shallow=False) for f in files) + + +def test_update_module_with_nextflow_config(self): + """Try updating a module with a nextflow.config file""" + # Install the module + assert self.mods_install.install("trimgalore") + # Add a nextflow.config file to the module + trimgalore_path = Path(self.pipeline_dir, "modules", "nf-core", "trimgalore") + Path(trimgalore_path, "nextflow.config").touch() + with open(Path(trimgalore_path, "nextflow.config"), "w") as fh: + fh.write("params.my_param = 'my_value'\n") + # Update the module + update_obj = ModuleUpdate(self.pipeline_dir, show_diff=False) + assert update_obj.update("trimgalore") + # Check that the nextflow.config file is still there + assert Path(trimgalore_path, "nextflow.config").exists() + with open(Path(trimgalore_path, "nextflow.config")) as fh: + assert "params.my_param = 'my_value'" in fh.read() diff --git a/tests/test_modules.py b/tests/test_modules.py index f9c3b6f2a7..796b1d2538 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -259,6 +259,7 @@ def test_modulesrepo_class(self): test_update_different_branch_mix_modules_branch_test, test_update_different_branch_mixed_modules_main, test_update_different_branch_single_module, + test_update_module_with_nextflow_config, test_update_only_show_differences, test_update_only_show_differences_when_patch, test_update_with_config_dont_update, From 8f946530b71781466b340b97d2d3d4a98ff82dd4 Mon Sep 17 00:00:00 2001 From: mashehu Date: Mon, 19 Feb 2024 12:33:35 +0100 Subject: [PATCH 3/4] move all possible config files during update --- nf_core/components/update.py | 13 ++++++++----- tests/modules/update.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 841ed45659..46136bc8da 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -740,13 +740,16 @@ def move_files_from_tmp_dir(self, component: str, install_folder: str, repo_path files = [f.name for f in temp_component_dir.iterdir() if f.is_file()] pipeline_path = Path(self.dir, self.component_type, repo_path, component) - # check if both directories have the same file and warn if the compnent_dir has more files if pipeline_path.exists(): pipeline_files = [f.name for f in pipeline_path.iterdir() if f.is_file()] - if "nextflow.config" in pipeline_files: - log.debug(f"Moving '{component}/nextflow.config' to updated component") - shutil.move(Path(pipeline_path, "nextflow.config"), Path(temp_component_dir, "nextflow.config")) - files.append("nextflow.config") + # check if any *.config file exists in the pipeline + if any([f.endswith(".config") for f in pipeline_files]): + # move the *.config file to the temporary directory + config_files = [f for f in files if f.endswith(".config")] + for config_file in config_files: + log.debug(f"Moving '{component}/{config_file}' to updated component") + shutil.move(Path(pipeline_path, config_file), Path(temp_component_dir, config_file)) + files.append(config_file) else: log.debug(f"Creating new {self.component_type[:-1]} '{component}' in '{self.component_type}/{repo_path}'") diff --git a/tests/modules/update.py b/tests/modules/update.py index 80cd5c2eed..81eb85716e 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -378,19 +378,19 @@ def cmp_module(dir1, dir2): return all(filecmp.cmp(os.path.join(dir1, f), os.path.join(dir2, f), shallow=False) for f in files) -def test_update_module_with_nextflow_config(self): - """Try updating a module with a nextflow.config file""" +def test_update_module_with_extra_config_file(self): + """Try updating a module with a config file""" # Install the module assert self.mods_install.install("trimgalore") - # Add a nextflow.config file to the module + # Add a nextflow_test.config file to the module trimgalore_path = Path(self.pipeline_dir, "modules", "nf-core", "trimgalore") - Path(trimgalore_path, "nextflow.config").touch() - with open(Path(trimgalore_path, "nextflow.config"), "w") as fh: + Path(trimgalore_path, "nextflow_test.config").touch() + with open(Path(trimgalore_path, "nextflow_test.config"), "w") as fh: fh.write("params.my_param = 'my_value'\n") # Update the module update_obj = ModuleUpdate(self.pipeline_dir, show_diff=False) assert update_obj.update("trimgalore") - # Check that the nextflow.config file is still there - assert Path(trimgalore_path, "nextflow.config").exists() - with open(Path(trimgalore_path, "nextflow.config")) as fh: + # Check that the nextflow_test.config file is still there + assert Path(trimgalore_path, "nextflow_test.config").exists() + with open(Path(trimgalore_path, "nextflow_test.config")) as fh: assert "params.my_param = 'my_value'" in fh.read() From 6b65a01b6b2293f0013ba511a9375e2e7df776e1 Mon Sep 17 00:00:00 2001 From: mashehu Date: Mon, 19 Feb 2024 12:35:31 +0100 Subject: [PATCH 4/4] fix test name --- tests/test_modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_modules.py b/tests/test_modules.py index 796b1d2538..86639d3800 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -259,7 +259,7 @@ def test_modulesrepo_class(self): test_update_different_branch_mix_modules_branch_test, test_update_different_branch_mixed_modules_main, test_update_different_branch_single_module, - test_update_module_with_nextflow_config, + test_update_module_with_extra_config_file, test_update_only_show_differences, test_update_only_show_differences_when_patch, test_update_with_config_dont_update,