From b3fdaf16249e2adcd67aed9e68c40417ac6b856a Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Tue, 30 Jan 2024 13:26:28 +0100 Subject: [PATCH 01/17] fix ignoring files_unchanged --- nf_core/lint/files_unchanged.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 176b0e9e65..3a96438b5a 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -153,7 +153,7 @@ def _tf(file_path: Union[str, Path]) -> Path: for files in files_exact: # Ignore if file specified in linting config ignore_files = self.lint_config.get("files_unchanged", []) - if any([f in ignore_files for f in files]): + if any([str(f) in ignore_files for f in files]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}") # Ignore if we can't find the file @@ -218,7 +218,7 @@ def _tf(file_path: Union[str, Path]) -> Path: for files in files_conditional: # Ignore if file specified in linting config ignore_files = self.lint_config.get("files_unchanged", []) - if files[0] in ignore_files: + if str(files[0]) in ignore_files: ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}") # Ignore if we can't find the file From c4be616bb6c6c3fdaf7143ebae10af296699f225 Mon Sep 17 00:00:00 2001 From: nf-core-bot Date: Tue, 30 Jan 2024 12:28:24 +0000 Subject: [PATCH 02/17] [automated] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae1f898184..4b6d48ce92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Linting - Handle default values of type number from nextflow schema ([#2703](https://github.com/nf-core/tools/pull/2703)) +- fix ignoring files_unchanged ([#2707](https://github.com/nf-core/tools/pull/2707)) ### Modules From 06a4c32da8a1326199b1faa36135e8302e28e9cd Mon Sep 17 00:00:00 2001 From: mashehu Date: Tue, 30 Jan 2024 14:35:18 +0100 Subject: [PATCH 03/17] fix types in files_unchanged --- nf_core/lint/__init__.py | 6 ++- nf_core/lint/files_unchanged.py | 76 +++++++++++++++++---------------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 4f7657aec5..143a5f18ef 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -8,6 +8,8 @@ import json import logging import os +from pathlib import Path +from typing import List, Union import git import rich @@ -621,7 +623,7 @@ def _save_json_results(self, json_fn): with open(json_fn, "w") as fh: json.dump(results, fh, indent=4) - def _wrap_quotes(self, files): + def _wrap_quotes(self, files: Union[List[str], List[Path]]) -> str: """Helper function to take a list of filenames and format with markdown. Args: @@ -636,5 +638,5 @@ def _wrap_quotes(self, files): """ if not isinstance(files, list): files = [files] - bfiles = [f"`{f}`" for f in files] + bfiles = [f"`{str(f)}`" for f in files] return " or ".join(bfiles) diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 3a96438b5a..beb60399ef 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -3,16 +3,17 @@ import shutil import tempfile from pathlib import Path -from typing import Union +from typing import Dict, List, Tuple, Union import yaml import nf_core.create +from nf_core.lint import PipelineLint log = logging.getLogger(__name__) -def files_unchanged(self): +def files_unchanged(self: PipelineLint) -> dict[str, Union[List[str], bool]]: """Checks that certain pipeline files are not modified from template output. Iterates through the pipeline's directory content and compares specified files @@ -64,11 +65,11 @@ def files_unchanged(self): """ - passed = [] - failed = [] - ignored = [] - fixed = [] - could_fix = False + passed: List[str] = [] + failed: List[str] = [] + ignored: List[str] = [] + fixed: List[str] = [] + could_fix: bool = False # Check that we have the minimum required config required_pipeline_config = {"manifest.name", "manifest.description", "manifest.author"} @@ -87,10 +88,10 @@ def files_unchanged(self): # NB: Should all be files, not directories # List of lists. Passes if any of the files in the sublist are found. files_exact = [ - [".gitattributes"], - [".prettierrc.yml"], - ["CODE_OF_CONDUCT.md"], - ["LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md"], # NB: British / American spelling + [Path(".gitattributes")], + [Path(".prettierrc.yml")], + [Path("CODE_OF_CONDUCT.md")], + [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")], # NB: British / American spelling [Path(".github", ".dockstore.yml")], [Path(".github", "CONTRIBUTING.md")], [Path(".github", "ISSUE_TEMPLATE", "bug_report.yml")], @@ -110,10 +111,10 @@ def files_unchanged(self): [Path("lib", "NfcoreTemplate.groovy")], ] files_partial = [ - [".gitignore", ".prettierignore", "pyproject.toml"], + [Path(".gitignore"), Path(".prettierignore"), Path("pyproject.toml")], ] - files_conditional = [ - [Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf_validation"}], + files_conditional: List[Tuple[List[Path], Dict[str, str]]] = [ + ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf_validation"}), ] # Only show error messages from pipeline creation @@ -215,37 +216,38 @@ def _tf(file_path: Union[str, Path]) -> Path: pass # Files that should be there only if an entry in nextflow config is not set - for files in files_conditional: + for file_conditional in files_conditional: # Ignore if file specified in linting config ignore_files = self.lint_config.get("files_unchanged", []) - if str(files[0]) in ignore_files: - ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}") + if any(str(f) in ignore_files for f in files_conditional[0]): + ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}") # Ignore if we can't find the file - elif _pf(files[0]).is_file(): - ignored.append(f"File does not exist: {self._wrap_quotes(files[0])}") + elif not any([_pf(f).is_file() for f in file_conditional[0]]): + ignored.append(f"File does not exist: {self._wrap_quotes(file_conditional[0])}") # Check that the file has an identical match else: - config_key, config_value = list(files[1].items())[0] - if config_key in self.nf_config and self.nf_config[config_key] == config_value: - # Ignore if the config key is set to the expected value - ignored.append(f"File ignored due to config: {self._wrap_quotes(files)}") - else: - try: - if filecmp.cmp(_pf(files[0]), _tf(files[0]), shallow=True): - passed.append(f"`{files[0]}` matches the template") - else: - if "files_unchanged" in self.fix: - # Try to fix the problem by overwriting the pipeline file - shutil.copy(_tf(files[0]), _pf(files[0])) - passed.append(f"`{files[0]}` matches the template") - fixed.append(f"`{files[0]}` overwritten with template file") + config_key, config_value = list(file_conditional[1].items())[0] + for f in file_conditional[0]: + if config_key in self.nf_config and self.nf_config[config_key] == config_value: + # Ignore if the config key is set to the expected value + ignored.append(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") + else: + try: + if filecmp.cmp(_pf(f), _tf(f), shallow=True): + passed.append(f"`{f}` matches the template") else: - failed.append(f"`{files[0]}` does not match the template") - could_fix = True - except FileNotFoundError: - pass + if "files_unchanged" in self.fix: + # Try to fix the problem by overwriting the pipeline file + shutil.copy(_tf(f), _pf(f)) + passed.append(f"`{f}` matches the template") + fixed.append(f"`{f}` overwritten with template file") + else: + failed.append(f"`{f}` does not match the template") + could_fix = True + except FileNotFoundError: + pass # cleaning up temporary dir shutil.rmtree(tmp_dir) From e0cf4eb84c2f06b47cb3e65b0b69fc727ff8362a Mon Sep 17 00:00:00 2001 From: mashehu Date: Tue, 30 Jan 2024 14:53:04 +0100 Subject: [PATCH 04/17] fix typing in files_exist --- nf_core/lint/__init__.py | 2 +- nf_core/lint/files_exist.py | 83 +++++++++++++++++++------------------ 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 143a5f18ef..d39ce5d7e9 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -623,7 +623,7 @@ def _save_json_results(self, json_fn): with open(json_fn, "w") as fh: json.dump(results, fh, indent=4) - def _wrap_quotes(self, files: Union[List[str], List[Path]]) -> str: + def _wrap_quotes(self, files: Union[List[str], List[Path], Path]) -> str: """Helper function to take a list of filenames and format with markdown. Args: diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index 78a744a4d7..aab6eebae2 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -1,11 +1,13 @@ import logging from pathlib import Path -from typing import Union +from typing import Dict, List, Tuple, Union + +from nf_core.lint import PipelineLint log = logging.getLogger(__name__) -def files_exist(self): +def files_exist(self: PipelineLint) -> dict[str, Union[List[str], bool]]: """Checks a given pipeline directory for required files. Iterates through the pipeline's directory content and checks that specified @@ -129,19 +131,19 @@ def files_exist(self): short_name = self.nf_config["manifest.name"].strip("\"'").split("/") files_fail = [ - [".gitattributes"], - [".gitignore"], - [".nf-core.yml"], - [".editorconfig"], - [".prettierignore"], - [".prettierrc.yml"], - ["CHANGELOG.md"], - ["CITATIONS.md"], - ["CODE_OF_CONDUCT.md"], - ["LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md"], # NB: British / American spelling - ["nextflow_schema.json"], - ["nextflow.config"], - ["README.md"], + [Path(".gitattributes")], + [Path(".gitignore")], + [Path(".nf-core.yml")], + [Path(".editorconfig")], + [Path(".prettierignore")], + [Path(".prettierrc.yml")], + [Path("CHANGELOG.md")], + [Path("CITATIONS.md")], + [Path("CODE_OF_CONDUCT.md")], + [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")], # NB: British / American spelling + [Path("nextflow_schema.json")], + [Path("nextflow.config")], + [Path("README.md")], [Path(".github", ".dockstore.yml")], [Path(".github", "CONTRIBUTING.md")], [Path(".github", "ISSUE_TEMPLATE", "bug_report.yml")], @@ -171,37 +173,39 @@ def files_exist(self): ] files_warn = [ - ["main.nf"], + [Path("main.nf")], [Path("assets", "multiqc_config.yml")], [Path("conf", "base.config")], [Path("conf", "igenomes.config")], [Path(".github", "workflows", "awstest.yml")], [Path(".github", "workflows", "awsfulltest.yml")], [Path("lib", f"Workflow{short_name[0].upper()}{short_name[1:]}.groovy")], - ["modules.json"], - ["pyproject.toml"], + [Path("modules.json")], + [Path("pyproject.toml")], ] # List of strings. Fails / warns if any of the strings exist. files_fail_ifexists = [ - "Singularity", - "parameters.settings.json", - "pipeline_template.yml", # saving information in .nf-core.yml - ".nf-core.yaml", # yml not yaml + Path("Singularity"), + Path("parameters.settings.json"), + Path("pipeline_template.yml"), # saving information in .nf-core.yml + Path(".nf-core.yaml"), # yml not yaml Path("bin", "markdown_to_html.r"), Path("conf", "aws.config"), Path(".github", "workflows", "push_dockerhub.yml"), Path(".github", "ISSUE_TEMPLATE", "bug_report.md"), Path(".github", "ISSUE_TEMPLATE", "feature_request.md"), Path("docs", "images", f"nf-core-{short_name}_logo.png"), - ".markdownlint.yml", - ".yamllint.yml", + Path(".markdownlint.yml"), + Path(".yamllint.yml"), Path("lib", "Checks.groovy"), Path("lib", "Completion.groovy"), Path("lib", "Workflow.groovy"), ] - files_warn_ifexists = [".travis.yml"] - files_fail_ifinconfig = [[Path("lib", "nfcore_external_java_deps.jar"), "nf-validation"]] + files_warn_ifexists = [Path(".travis.yml")] + files_fail_ifinconfig: List[Tuple[Path, Dict[str, str]]] = [ + (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf_validation"}), + ] # Remove files that should be ignored according to the linting config ignore_files = self.lint_config.get("files_exist", []) @@ -242,22 +246,21 @@ def pf(file_path: Union[str, Path]) -> Path: else: passed.append(f"File not found check: {self._wrap_quotes(file)}") # Files that cause an error if they exists together with a certain entry in nextflow.config - for file in files_fail_ifinconfig: - if str(file[0]) in ignore_files: + for file_cond in files_fail_ifinconfig: + if str(file_cond[0]) in ignore_files: continue - nextflow_config = pf("nextflow.config") in_config = False - with open(nextflow_config) as f: - if file[1] in f.read(): - in_config = True - if pf(file[0]).is_file() and in_config: - failed.append(f"File must be removed: {self._wrap_quotes(file[0])}") - elif pf(file[0]).is_file() and not in_config: - passed.append(f"File found check: {self._wrap_quotes(file[0])}") - elif not pf(file[0]).is_file() and not in_config: - failed.append(f"File not found check: {self._wrap_quotes(file[0])}") - elif not pf(file[0]).is_file() and in_config: - passed.append(f"File not found check: {self._wrap_quotes(file[0])}") + config_key, config_value = list(file_cond[1].items())[0] + if config_key in self.nf_config and self.nf_config[config_key] == config_value: + in_config = True + if pf(file_cond[0]).is_file() and in_config: + failed.append(f"File must be removed: {self._wrap_quotes(file_cond[0])}") + elif pf(file_cond[0]).is_file() and not in_config: + passed.append(f"File found check: {self._wrap_quotes(file_cond[0])}") + elif not pf(file_cond[0]).is_file() and not in_config: + failed.append(f"File not found check: {self._wrap_quotes(file_cond[0])}") + elif not pf(file_cond[0]).is_file() and in_config: + passed.append(f"File not found check: {self._wrap_quotes(file_cond[0])}") # Files that cause a warning if they exist for file in files_warn_ifexists: if file in ignore_files: From f7827f60c86776e9199d097e014570103c61687d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20H=C3=B6rtenhuber?= Date: Tue, 30 Jan 2024 15:20:14 +0100 Subject: [PATCH 05/17] avoid circular import --- nf_core/lint/files_exist.py | 3 +-- nf_core/lint/files_unchanged.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index aab6eebae2..b90a7d14e7 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -2,12 +2,11 @@ from pathlib import Path from typing import Dict, List, Tuple, Union -from nf_core.lint import PipelineLint log = logging.getLogger(__name__) -def files_exist(self: PipelineLint) -> dict[str, Union[List[str], bool]]: +def files_exist(self) -> dict[str, Union[List[str], bool]]: """Checks a given pipeline directory for required files. Iterates through the pipeline's directory content and checks that specified diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index beb60399ef..c868d1af67 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -8,12 +8,11 @@ import yaml import nf_core.create -from nf_core.lint import PipelineLint log = logging.getLogger(__name__) -def files_unchanged(self: PipelineLint) -> dict[str, Union[List[str], bool]]: +def files_unchanged(self) -> dict[str, Union[List[str], bool]]: """Checks that certain pipeline files are not modified from template output. Iterates through the pipeline's directory content and compares specified files From 84108922fe3c26eb5199bda1dbbc083e1307c656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20H=C3=B6rtenhuber?= Date: Tue, 30 Jan 2024 14:35:26 +0000 Subject: [PATCH 06/17] fix linting --- nf_core/lint/files_exist.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index b90a7d14e7..f6a041d992 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -2,7 +2,6 @@ from pathlib import Path from typing import Dict, List, Tuple, Union - log = logging.getLogger(__name__) From d41694bd5b2bf1afa4f72cd7996a1cee48a18db1 Mon Sep 17 00:00:00 2001 From: mashehu Date: Tue, 30 Jan 2024 15:58:57 +0100 Subject: [PATCH 07/17] fix plugin entry --- .github/workflows/fix-linting.yml | 2 +- nf_core/lint/files_exist.py | 9 ++++++--- nf_core/lint/files_unchanged.py | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/fix-linting.yml b/.github/workflows/fix-linting.yml index 595fff66ae..95a03c70fe 100644 --- a/.github/workflows/fix-linting.yml +++ b/.github/workflows/fix-linting.yml @@ -10,7 +10,7 @@ jobs: contains(github.event.comment.html_url, '/pull/') && contains(github.event.comment.body, '@nf-core-bot fix linting') && github.repository == 'nf-core/tools' - runs-on: self-hosted + runs-on: ubuntu-latest steps: # Use the @nf-core-bot token to check out so we can push later - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4 diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index f6a041d992..dec96156df 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -138,7 +138,7 @@ def files_exist(self) -> dict[str, Union[List[str], bool]]: [Path("CHANGELOG.md")], [Path("CITATIONS.md")], [Path("CODE_OF_CONDUCT.md")], - [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")], # NB: British / American spelling + [Path("LICENSE"), Path("LICENSE.md"), Path("LICENCE"), Path("LICENCE.md")], # NB: British / American spelling [Path("nextflow_schema.json")], [Path("nextflow.config")], [Path("README.md")], @@ -202,7 +202,7 @@ def files_exist(self) -> dict[str, Union[List[str], bool]]: ] files_warn_ifexists = [Path(".travis.yml")] files_fail_ifinconfig: List[Tuple[Path, Dict[str, str]]] = [ - (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf_validation"}), + (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf-validation"}), ] # Remove files that should be ignored according to the linting config @@ -245,11 +245,14 @@ def pf(file_path: Union[str, Path]) -> Path: passed.append(f"File not found check: {self._wrap_quotes(file)}") # Files that cause an error if they exists together with a certain entry in nextflow.config for file_cond in files_fail_ifinconfig: + log.error(f"{self.nf_config=}") if str(file_cond[0]) in ignore_files: continue in_config = False config_key, config_value = list(file_cond[1].items())[0] - if config_key in self.nf_config and self.nf_config[config_key] == config_value: + if config_key in self.nf_config and config_value in self.nf_config[config_key]: + in_config = True + log.debug(f"Found {config_key} in nextflow.config with value {config_value}") in_config = True if pf(file_cond[0]).is_file() and in_config: failed.append(f"File must be removed: {self._wrap_quotes(file_cond[0])}") diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index c868d1af67..4e106d156b 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -113,7 +113,7 @@ def files_unchanged(self) -> dict[str, Union[List[str], bool]]: [Path(".gitignore"), Path(".prettierignore"), Path("pyproject.toml")], ] files_conditional: List[Tuple[List[Path], Dict[str, str]]] = [ - ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf_validation"}), + ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf-validation"}), ] # Only show error messages from pipeline creation @@ -229,7 +229,7 @@ def _tf(file_path: Union[str, Path]) -> Path: else: config_key, config_value = list(file_conditional[1].items())[0] for f in file_conditional[0]: - if config_key in self.nf_config and self.nf_config[config_key] == config_value: + if config_key in self.nf_config and config_value in self.nf_config[config_key]: # Ignore if the config key is set to the expected value ignored.append(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") else: From 20c8203ecd25227864cfa4bc592a96ded58fa95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20H=C3=B6rtenhuber?= Date: Tue, 30 Jan 2024 15:03:18 +0000 Subject: [PATCH 08/17] fix plugin entry --- .github/workflows/fix-linting.yml | 2 +- nf_core/lint/files_exist.py | 9 ++++++--- nf_core/lint/files_unchanged.py | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/fix-linting.yml b/.github/workflows/fix-linting.yml index 595fff66ae..95a03c70fe 100644 --- a/.github/workflows/fix-linting.yml +++ b/.github/workflows/fix-linting.yml @@ -10,7 +10,7 @@ jobs: contains(github.event.comment.html_url, '/pull/') && contains(github.event.comment.body, '@nf-core-bot fix linting') && github.repository == 'nf-core/tools' - runs-on: self-hosted + runs-on: ubuntu-latest steps: # Use the @nf-core-bot token to check out so we can push later - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4 diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index f6a041d992..dec96156df 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -138,7 +138,7 @@ def files_exist(self) -> dict[str, Union[List[str], bool]]: [Path("CHANGELOG.md")], [Path("CITATIONS.md")], [Path("CODE_OF_CONDUCT.md")], - [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")], # NB: British / American spelling + [Path("LICENSE"), Path("LICENSE.md"), Path("LICENCE"), Path("LICENCE.md")], # NB: British / American spelling [Path("nextflow_schema.json")], [Path("nextflow.config")], [Path("README.md")], @@ -202,7 +202,7 @@ def files_exist(self) -> dict[str, Union[List[str], bool]]: ] files_warn_ifexists = [Path(".travis.yml")] files_fail_ifinconfig: List[Tuple[Path, Dict[str, str]]] = [ - (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf_validation"}), + (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf-validation"}), ] # Remove files that should be ignored according to the linting config @@ -245,11 +245,14 @@ def pf(file_path: Union[str, Path]) -> Path: passed.append(f"File not found check: {self._wrap_quotes(file)}") # Files that cause an error if they exists together with a certain entry in nextflow.config for file_cond in files_fail_ifinconfig: + log.error(f"{self.nf_config=}") if str(file_cond[0]) in ignore_files: continue in_config = False config_key, config_value = list(file_cond[1].items())[0] - if config_key in self.nf_config and self.nf_config[config_key] == config_value: + if config_key in self.nf_config and config_value in self.nf_config[config_key]: + in_config = True + log.debug(f"Found {config_key} in nextflow.config with value {config_value}") in_config = True if pf(file_cond[0]).is_file() and in_config: failed.append(f"File must be removed: {self._wrap_quotes(file_cond[0])}") diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index c868d1af67..4e106d156b 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -113,7 +113,7 @@ def files_unchanged(self) -> dict[str, Union[List[str], bool]]: [Path(".gitignore"), Path(".prettierignore"), Path("pyproject.toml")], ] files_conditional: List[Tuple[List[Path], Dict[str, str]]] = [ - ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf_validation"}), + ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf-validation"}), ] # Only show error messages from pipeline creation @@ -229,7 +229,7 @@ def _tf(file_path: Union[str, Path]) -> Path: else: config_key, config_value = list(file_conditional[1].items())[0] for f in file_conditional[0]: - if config_key in self.nf_config and self.nf_config[config_key] == config_value: + if config_key in self.nf_config and config_value in self.nf_config[config_key]: # Ignore if the config key is set to the expected value ignored.append(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") else: From 81a3f7a2be5ba556a92116e38e515ab0977378a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Tue, 30 Jan 2024 16:16:38 +0100 Subject: [PATCH 09/17] Update nf_core/lint/files_exist.py --- nf_core/lint/files_exist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index dec96156df..e2814ee6dc 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -264,7 +264,7 @@ def pf(file_path: Union[str, Path]) -> Path: passed.append(f"File not found check: {self._wrap_quotes(file_cond[0])}") # Files that cause a warning if they exist for file in files_warn_ifexists: - if file in ignore_files: + if str(file) in ignore_files: continue if pf(file).is_file(): warned.append(f"File should be removed: {self._wrap_quotes(file)}") From 4f6522a8918c8c1727ba8999966446b728026f9b Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 10:23:41 +0100 Subject: [PATCH 10/17] fix paths, ignored listing and remove debugging --- nf_core/lint/files_exist.py | 2 -- nf_core/lint/files_unchanged.py | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index e2814ee6dc..fecf7e2595 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -207,7 +207,6 @@ def files_exist(self) -> dict[str, Union[List[str], bool]]: # Remove files that should be ignored according to the linting config ignore_files = self.lint_config.get("files_exist", []) - log.info(f"Files to ignore: {ignore_files}") def pf(file_path: Union[str, Path]) -> Path: return Path(self.wf_path, file_path) @@ -245,7 +244,6 @@ def pf(file_path: Union[str, Path]) -> Path: passed.append(f"File not found check: {self._wrap_quotes(file)}") # Files that cause an error if they exists together with a certain entry in nextflow.config for file_cond in files_fail_ifinconfig: - log.error(f"{self.nf_config=}") if str(file_cond[0]) in ignore_files: continue in_config = False diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 4e106d156b..d3fd5b8302 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -90,7 +90,7 @@ def files_unchanged(self) -> dict[str, Union[List[str], bool]]: [Path(".gitattributes")], [Path(".prettierrc.yml")], [Path("CODE_OF_CONDUCT.md")], - [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")], # NB: British / American spelling + [Path("LICENSE"), Path("LICENSE.md"), Path("LICENCE"), Path("LICENCE.md")], # NB: British / American spelling [Path(".github", ".dockstore.yml")], [Path(".github", "CONTRIBUTING.md")], [Path(".github", "ISSUE_TEMPLATE", "bug_report.yml")], @@ -221,10 +221,6 @@ def _tf(file_path: Union[str, Path]) -> Path: if any(str(f) in ignore_files for f in files_conditional[0]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}") - # Ignore if we can't find the file - elif not any([_pf(f).is_file() for f in file_conditional[0]]): - ignored.append(f"File does not exist: {self._wrap_quotes(file_conditional[0])}") - # Check that the file has an identical match else: config_key, config_value = list(file_conditional[1].items())[0] From c0d6bc238006d044f1537b3302fcb716ce3870b1 Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 11:06:38 +0100 Subject: [PATCH 11/17] fix types --- nf_core/lint/files_exist.py | 2 +- nf_core/lint/files_unchanged.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index fecf7e2595..6f6d0a77d9 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -5,7 +5,7 @@ log = logging.getLogger(__name__) -def files_exist(self) -> dict[str, Union[List[str], bool]]: +def files_exist(self) -> Dict[str, Union[List[str], bool]]: """Checks a given pipeline directory for required files. Iterates through the pipeline's directory content and checks that specified diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index d3fd5b8302..589e5dc05b 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -12,7 +12,7 @@ log = logging.getLogger(__name__) -def files_unchanged(self) -> dict[str, Union[List[str], bool]]: +def files_unchanged(self) -> Dict[str, Union[List[str], bool]]: """Checks that certain pipeline files are not modified from template output. Iterates through the pipeline's directory content and compares specified files From 2cb07f4389ce5d7eba0aa957d271ee71e739dd9f Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 11:53:23 +0100 Subject: [PATCH 12/17] fix reporting of ignored conditional files --- nf_core/lint/files_unchanged.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 589e5dc05b..6dcd87d320 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -149,10 +149,11 @@ def _tf(file_path: Union[str, Path]) -> Path: """Helper function - get file path for template file""" return Path(test_pipeline_dir, file_path) + ignore_files = self.lint_config.get("files_unchanged", []) + # Files that must be completely unchanged from template for files in files_exact: # Ignore if file specified in linting config - ignore_files = self.lint_config.get("files_unchanged", []) if any([str(f) in ignore_files for f in files]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}") @@ -181,7 +182,6 @@ def _tf(file_path: Union[str, Path]) -> Path: # Files that can be added to, but that must contain the template contents for files in files_partial: # Ignore if file specified in linting config - ignore_files = self.lint_config.get("files_unchanged", []) if any([f in ignore_files for f in files]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}") @@ -217,8 +217,7 @@ def _tf(file_path: Union[str, Path]) -> Path: # Files that should be there only if an entry in nextflow config is not set for file_conditional in files_conditional: # Ignore if file specified in linting config - ignore_files = self.lint_config.get("files_unchanged", []) - if any(str(f) in ignore_files for f in files_conditional[0]): + if any([f in ignore_files for f in file_conditional[0]]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}") # Check that the file has an identical match @@ -227,7 +226,7 @@ def _tf(file_path: Union[str, Path]) -> Path: for f in file_conditional[0]: if config_key in self.nf_config and config_value in self.nf_config[config_key]: # Ignore if the config key is set to the expected value - ignored.append(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") + log.debug(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") else: try: if filecmp.cmp(_pf(f), _tf(f), shallow=True): From 18270330dbc035851a33fc3b1fcf9a3df5b75f5b Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 11:59:21 +0100 Subject: [PATCH 13/17] rescue removed ignore statement by logging it directly --- nf_core/lint/files_unchanged.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 6dcd87d320..62c864bbf0 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -219,7 +219,9 @@ def _tf(file_path: Union[str, Path]) -> Path: # Ignore if file specified in linting config if any([f in ignore_files for f in file_conditional[0]]): ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}") - + # Ignore if we can't find the file + elif not any([_pf(f).is_file() for f in file_conditional[0]]): + log.debug(f"File does not exist: {self._wrap_quotes(file_conditional[0])}") # Check that the file has an identical match else: config_key, config_value = list(file_conditional[1].items())[0] From 5893c225e9db47acffd3d37ecbd5357141617cac Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 12:32:58 +0100 Subject: [PATCH 14/17] actually don't need conditional in files unchanged if they are not anymore in the template --- nf_core/lint/files_unchanged.py | 39 +-------------------------------- tests/lint/files_unchanged.py | 8 +++---- 2 files changed, 5 insertions(+), 42 deletions(-) diff --git a/nf_core/lint/files_unchanged.py b/nf_core/lint/files_unchanged.py index 62c864bbf0..399830faae 100644 --- a/nf_core/lint/files_unchanged.py +++ b/nf_core/lint/files_unchanged.py @@ -3,7 +3,7 @@ import shutil import tempfile from pathlib import Path -from typing import Dict, List, Tuple, Union +from typing import Dict, List, Union import yaml @@ -49,9 +49,6 @@ def files_unchanged(self) -> Dict[str, Union[List[str], bool]]: .prettierignore pyproject.toml - Files that need to be there or not based on a entry in nextflow config:: - - lib/nfcore_external_java_deps.jar # if config doesn't mention nf-validation .. tip:: You can configure the ``nf-core lint`` tests to ignore any of these checks by setting the ``files_unchanged`` key as follows in your ``.nf-core.yml`` config file. For example: @@ -112,9 +109,6 @@ def files_unchanged(self) -> Dict[str, Union[List[str], bool]]: files_partial = [ [Path(".gitignore"), Path(".prettierignore"), Path("pyproject.toml")], ] - files_conditional: List[Tuple[List[Path], Dict[str, str]]] = [ - ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf-validation"}), - ] # Only show error messages from pipeline creation logging.getLogger("nf_core.create").setLevel(logging.ERROR) @@ -214,37 +208,6 @@ def _tf(file_path: Union[str, Path]) -> Path: except FileNotFoundError: pass - # Files that should be there only if an entry in nextflow config is not set - for file_conditional in files_conditional: - # Ignore if file specified in linting config - if any([f in ignore_files for f in file_conditional[0]]): - ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}") - # Ignore if we can't find the file - elif not any([_pf(f).is_file() for f in file_conditional[0]]): - log.debug(f"File does not exist: {self._wrap_quotes(file_conditional[0])}") - # Check that the file has an identical match - else: - config_key, config_value = list(file_conditional[1].items())[0] - for f in file_conditional[0]: - if config_key in self.nf_config and config_value in self.nf_config[config_key]: - # Ignore if the config key is set to the expected value - log.debug(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}") - else: - try: - if filecmp.cmp(_pf(f), _tf(f), shallow=True): - passed.append(f"`{f}` matches the template") - else: - if "files_unchanged" in self.fix: - # Try to fix the problem by overwriting the pipeline file - shutil.copy(_tf(f), _pf(f)) - passed.append(f"`{f}` matches the template") - fixed.append(f"`{f}` overwritten with template file") - else: - failed.append(f"`{f}` does not match the template") - could_fix = True - except FileNotFoundError: - pass - # cleaning up temporary dir shutil.rmtree(tmp_dir) diff --git a/tests/lint/files_unchanged.py b/tests/lint/files_unchanged.py index 84aec50c26..601f09b9df 100644 --- a/tests/lint/files_unchanged.py +++ b/tests/lint/files_unchanged.py @@ -1,4 +1,4 @@ -import os +from pathlib import Path import nf_core.lint @@ -13,14 +13,14 @@ def test_files_unchanged_pass(self): def test_files_unchanged_fail(self): - failing_file = os.path.join(".github", "CONTRIBUTING.md") + failing_file = Path(".github", "CONTRIBUTING.md") new_pipeline = self._make_pipeline_copy() - with open(os.path.join(new_pipeline, failing_file), "a") as fh: + with open(Path(new_pipeline, failing_file), "a") as fh: fh.write("THIS SHOULD NOT BE HERE") lint_obj = nf_core.lint.PipelineLint(new_pipeline) lint_obj._load() results = lint_obj.files_unchanged() assert len(results["failed"]) > 0 - assert failing_file in results["failed"][0] + assert str(failing_file) in results["failed"][0] assert results["could_fix"] From 59cbb3f59f28b4938ce4a7301daf0885407a0b08 Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 12:33:19 +0100 Subject: [PATCH 15/17] add tests for conditionals --- nf_core/lint/files_exist.py | 3 +-- tests/lint/files_exist.py | 21 +++++++++++++++++++++ tests/test_lint.py | 1 + 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/nf_core/lint/files_exist.py b/nf_core/lint/files_exist.py index 6f6d0a77d9..1bd6dba74c 100644 --- a/nf_core/lint/files_exist.py +++ b/nf_core/lint/files_exist.py @@ -249,7 +249,6 @@ def pf(file_path: Union[str, Path]) -> Path: in_config = False config_key, config_value = list(file_cond[1].items())[0] if config_key in self.nf_config and config_value in self.nf_config[config_key]: - in_config = True log.debug(f"Found {config_key} in nextflow.config with value {config_value}") in_config = True if pf(file_cond[0]).is_file() and in_config: @@ -269,7 +268,7 @@ def pf(file_path: Union[str, Path]) -> Path: else: passed.append(f"File not found check: {self._wrap_quotes(file)}") - # Files that are ignoed + # Files that are ignored for file in ignore_files: ignored.append(f"File is ignored: {self._wrap_quotes(file)}") diff --git a/tests/lint/files_exist.py b/tests/lint/files_exist.py index 5ba26d77a0..d3a6a25e82 100644 --- a/tests/lint/files_exist.py +++ b/tests/lint/files_exist.py @@ -54,3 +54,24 @@ def test_files_exist_pass(self): results = lint_obj.files_exist() assert results["failed"] == [] + + +def test_files_exist_pass_conditional(self): + new_pipeline = self._make_pipeline_copy() + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + lint_obj.nf_config["plugins"] = [] + Path(new_pipeline, "lib/nfcore_external_java_deps.jar").touch() + results = lint_obj.files_exist() + assert results["failed"] == [] + assert results["ignored"] == [] + + +def test_files_exist_fail_conditional(self): + new_pipeline = self._make_pipeline_copy() + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + Path(new_pipeline, "lib/nfcore_external_java_deps.jar").touch() + results = lint_obj.files_exist() + assert results["failed"] == ["File must be removed: `lib/nfcore_external_java_deps.jar`"] + assert results["ignored"] == [] diff --git a/tests/test_lint.py b/tests/test_lint.py index c8d7135654..be18b87910 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -200,6 +200,7 @@ def test_sphinx_md_files(self): ) from .lint.files_exist import ( # type: ignore[misc] test_files_exist_depreciated_file, + test_files_exist_fail_conditional, test_files_exist_missing_config, test_files_exist_missing_main, test_files_exist_pass, From c445b936e89bb29796f17e61f1e71b366b5c1618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20H=C3=B6rtenhuber?= Date: Wed, 31 Jan 2024 13:23:30 +0100 Subject: [PATCH 16/17] Update tests/test_lint.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- tests/test_lint.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_lint.py b/tests/test_lint.py index be18b87910..ee68648ce6 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -201,6 +201,7 @@ def test_sphinx_md_files(self): from .lint.files_exist import ( # type: ignore[misc] test_files_exist_depreciated_file, test_files_exist_fail_conditional, + test_files_exist_pass_conditional, test_files_exist_missing_config, test_files_exist_missing_main, test_files_exist_pass, From 09a3514a3242f5f6690e329c980f92dd7c19d610 Mon Sep 17 00:00:00 2001 From: mashehu Date: Wed, 31 Jan 2024 13:36:35 +0100 Subject: [PATCH 17/17] fix linting --- tests/test_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_lint.py b/tests/test_lint.py index ee68648ce6..2f989aeffb 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -201,10 +201,10 @@ def test_sphinx_md_files(self): from .lint.files_exist import ( # type: ignore[misc] test_files_exist_depreciated_file, test_files_exist_fail_conditional, - test_files_exist_pass_conditional, test_files_exist_missing_config, test_files_exist_missing_main, test_files_exist_pass, + test_files_exist_pass_conditional, ) from .lint.files_unchanged import ( # type: ignore[misc] test_files_unchanged_fail,