From d47ba22a097733a4be94dee1cee78d09ef80860c Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 25 Jan 2024 16:26:43 +0100 Subject: [PATCH 1/7] allow ignoring specific files when linting template_strings --- nf_core/lint/template_strings.py | 25 ++++++++++++++++++++++++- nf_core/utils.py | 6 +++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/nf_core/lint/template_strings.py b/nf_core/lint/template_strings.py index 3467229362..4b5ebb6d72 100644 --- a/nf_core/lint/template_strings.py +++ b/nf_core/lint/template_strings.py @@ -16,13 +16,36 @@ def template_strings(self): This test ignores any double-brackets prefixed with a dollar sign, such as ``${{ secrets.AWS_ACCESS_KEY_ID }}`` as these placeholders are used in GitHub Actions workflows. + + .. tip:: You can choose to ignore this test tests by creating a file called + ``.nf-core.yml`` in the root of your pipeline and setting the test to false: + + .. code-block:: yaml + + lint: + template_strings: False + + To disable this test only for specific files, you can specify a list of file paths to ignore. + For exampke, to ignore a pdf you added to the docs: + + .. code-block:: yaml + + lint: + template_strings: + - docs/my_pdf.pdf """ passed = [] failed = [] + ignored = [] + # Files that should be ignored according to the linting config + ignore_files = self.lint_config.get("template_strings", []) # Loop through files, searching for string num_matches = 0 for fn in self.files: + if str(fn.relative_to(".")) in ignore_files: + ignored.append(f"Ignoring Jinja template strings in file `{fn}`") + continue # Skip binary files binary_ftypes = ["image", "application/java-archive"] (ftype, encoding) = mimetypes.guess_type(fn) @@ -41,4 +64,4 @@ def template_strings(self): if num_matches == 0: passed.append(f"Did not find any Jinja template strings ({len(self.files)} files)") - return {"passed": passed, "failed": failed} + return {"passed": passed, "failed": failed, "ignored": ignored} diff --git a/nf_core/utils.py b/nf_core/utils.py index 0b54056cfd..06b12c7ff4 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -159,8 +159,8 @@ def _list_files(self): git_ls_files = subprocess.check_output(["git", "ls-files"], cwd=self.wf_path).splitlines() self.files = [] for fn in git_ls_files: - full_fn = os.path.join(self.wf_path, fn.decode("utf-8")) - if os.path.isfile(full_fn): + full_fn = Path(self.wf_path) / fn.decode("utf-8") + if full_fn.is_file(): self.files.append(full_fn) else: log.debug(f"`git ls-files` returned '{full_fn}' but could not open it!") @@ -170,7 +170,7 @@ def _list_files(self): self.files = [] for subdir, _, files in os.walk(self.wf_path): for fn in files: - self.files.append(os.path.join(subdir, fn)) + self.files.append(Path(subdir) / fn) def _load_pipeline_config(self): """Get the nextflow config for this pipeline From a278b61a9cbf5faedb7dfa6c97bcba689613fc1e Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 25 Jan 2024 16:59:27 +0100 Subject: [PATCH 2/7] add tests for linting template_strings (not passing) --- nf_core/lint/template_strings.py | 2 +- tests/lint/template_strings.py | 59 ++++++++++++++++++++++++++++++++ tests/test_lint.py | 5 +++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/lint/template_strings.py diff --git a/nf_core/lint/template_strings.py b/nf_core/lint/template_strings.py index 4b5ebb6d72..5f7d644e40 100644 --- a/nf_core/lint/template_strings.py +++ b/nf_core/lint/template_strings.py @@ -43,7 +43,7 @@ def template_strings(self): # Loop through files, searching for string num_matches = 0 for fn in self.files: - if str(fn.relative_to(".")) in ignore_files: + if str(fn).removeprefix(".") in ignore_files: ignored.append(f"Ignoring Jinja template strings in file `{fn}`") continue # Skip binary files diff --git a/tests/lint/template_strings.py b/tests/lint/template_strings.py new file mode 100644 index 0000000000..280f3b48db --- /dev/null +++ b/tests/lint/template_strings.py @@ -0,0 +1,59 @@ +from pathlib import Path + +import nf_core.create +import nf_core.lint + + +def test_template_strings(self): + """Tests finding a template string in a file fails linting.""" + new_pipeline = self._make_pipeline_copy() + # Add template string to a file + file = Path(new_pipeline) / "docs" / "test.txt" + with open(file, "w") as f: + f.write("{{ template_string }}") + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + result = lint_obj.template_strings() + print(result["failed"]) + assert len(result["failed"]) == 1 + assert len(result["ignored"]) == 0 + + +def test_template_strings_ignored(self): + """Tests ignoring template_strings""" + new_pipeline = self._make_pipeline_copy() + # Add template string to a file + file = Path(new_pipeline) / "docs" / "test.txt" + with open(file, "w") as f: + f.write("{{ template_string }}") + # Ignore template_strings test + nf_core_yml = Path(new_pipeline) / ".nf-core.yml" + with open(nf_core_yml, "w") as f: + f.write("repository_type: pipeline\nlint:\n template_strings: False") + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + lint_obj._lint_pipeline() + assert len(lint_obj.failed) == 0 + assert len(lint_obj.ignored) == 1 + + +def test_template_strings_ignore_file(self): + """Tests ignoring template_strings file""" + new_pipeline = self._make_pipeline_copy() + # Add template string to a file + txt_file = Path(new_pipeline) / "docs" / "test.txt" + with open(txt_file, "w") as f: + f.write("{{ template_string }}") + # Ignore template_strings test + nf_core_yml = Path(new_pipeline) / ".nf-core.yml" + with open(nf_core_yml, "w") as f: + f.write("repository_type: pipeline\nlint:\n template_strings:\n - docs/test.txt") + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + lint_obj._list_files() + print(lint_obj.files) + result = lint_obj.template_strings() + print(result) + print(result["ignored"]) + assert len(result["failed"]) == 0 + assert len(result["ignored"]) == 1 diff --git a/tests/test_lint.py b/tests/test_lint.py index 32913bda0d..ee3014f727 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -224,6 +224,11 @@ def test_sphinx_md_files(self): test_nextflow_config_example_pass, test_nextflow_config_missing_test_profile_failed, ) + from .lint.template_strings import ( # type: ignore[misc] + test_template_strings, + test_template_strings_ignore_file, + test_template_strings_ignored, + ) from .lint.version_consistency import test_version_consistency # type: ignore[misc] From a67deb1e042a1468da88bd97bfd2a3d6c49fcae1 Mon Sep 17 00:00:00 2001 From: nf-core-bot Date: Thu, 25 Jan 2024 16:02:33 +0000 Subject: [PATCH 3/7] [automated] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ea7986b93..29bf3bc8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Fix linting of a pipeline with patched custom module ([#2669](https://github.com/nf-core/tools/pull/2669)) - linting a pipeline also lints the installed subworkflows ([#2677](https://github.com/nf-core/tools/pull/2677)) - environment.yml name must be lowercase ([#2676](https://github.com/nf-core/tools/pull/2676)) +- allow ignoring specific files when template_strings ([#2686](https://github.com/nf-core/tools/pull/2686)) ### Modules From 71b17d57e8a54ef5f3326a62c367eb1462bae242 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 26 Jan 2024 10:23:14 +0100 Subject: [PATCH 4/7] fix template_strings tests --- nf_core/lint/template_strings.py | 2 +- tests/lint/template_strings.py | 19 +++++++------------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/nf_core/lint/template_strings.py b/nf_core/lint/template_strings.py index 5f7d644e40..3e9cfd18fc 100644 --- a/nf_core/lint/template_strings.py +++ b/nf_core/lint/template_strings.py @@ -43,7 +43,7 @@ def template_strings(self): # Loop through files, searching for string num_matches = 0 for fn in self.files: - if str(fn).removeprefix(".") in ignore_files: + if str(fn.relative_to(self.wf_path)) in ignore_files: ignored.append(f"Ignoring Jinja template strings in file `{fn}`") continue # Skip binary files diff --git a/tests/lint/template_strings.py b/tests/lint/template_strings.py index 280f3b48db..ac0ae01681 100644 --- a/tests/lint/template_strings.py +++ b/tests/lint/template_strings.py @@ -1,3 +1,4 @@ +import subprocess from pathlib import Path import nf_core.create @@ -8,9 +9,10 @@ def test_template_strings(self): """Tests finding a template string in a file fails linting.""" new_pipeline = self._make_pipeline_copy() # Add template string to a file - file = Path(new_pipeline) / "docs" / "test.txt" - with open(file, "w") as f: - f.write("{{ template_string }}") + txt_file = Path(new_pipeline) / "docs" / "test.txt" + with open(txt_file, "w") as f: + f.write("my {{ template_string }}") + subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline) lint_obj = nf_core.lint.PipelineLint(new_pipeline) lint_obj._load() result = lint_obj.template_strings() @@ -22,10 +24,6 @@ def test_template_strings(self): def test_template_strings_ignored(self): """Tests ignoring template_strings""" new_pipeline = self._make_pipeline_copy() - # Add template string to a file - file = Path(new_pipeline) / "docs" / "test.txt" - with open(file, "w") as f: - f.write("{{ template_string }}") # Ignore template_strings test nf_core_yml = Path(new_pipeline) / ".nf-core.yml" with open(nf_core_yml, "w") as f: @@ -43,17 +41,14 @@ def test_template_strings_ignore_file(self): # Add template string to a file txt_file = Path(new_pipeline) / "docs" / "test.txt" with open(txt_file, "w") as f: - f.write("{{ template_string }}") + f.write("my {{ template_string }}") + subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline) # Ignore template_strings test nf_core_yml = Path(new_pipeline) / ".nf-core.yml" with open(nf_core_yml, "w") as f: f.write("repository_type: pipeline\nlint:\n template_strings:\n - docs/test.txt") lint_obj = nf_core.lint.PipelineLint(new_pipeline) lint_obj._load() - lint_obj._list_files() - print(lint_obj.files) result = lint_obj.template_strings() - print(result) - print(result["ignored"]) assert len(result["failed"]) == 0 assert len(result["ignored"]) == 1 From 4e233258e9910fdacce16885325e69124a38e4d9 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 26 Jan 2024 10:56:58 +0100 Subject: [PATCH 5/7] add docs for merge_markers --- nf_core/lint/merge_markers.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nf_core/lint/merge_markers.py b/nf_core/lint/merge_markers.py index 8ef425234b..a9b586a109 100644 --- a/nf_core/lint/merge_markers.py +++ b/nf_core/lint/merge_markers.py @@ -13,6 +13,17 @@ def merge_markers(self): This test looks for remaining merge markers in the code, e.g.: >>>>>>> or <<<<<<< + .. tip:: You can choose to ignore this test tests by creating a file called + ``.nf-core.yml`` in the root of your pipeline and setting the test to false: + .. code-block:: yaml + lint: + merge_markers: False + To disable this test only for specific files, you can specify a list of file paths to ignore. + For example, to ignore a pdf you added to the docs: + .. code-block:: yaml + lint: + merge_markers: + - docs/my_pdf.pdf """ passed = [] failed = [] From 5a25489fcdbcd522a376e67ab06316b608ce68a7 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 26 Jan 2024 16:07:32 +0100 Subject: [PATCH 6/7] fix tests failing due to Path --- tests/test_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 3079d75808..4b5ab19fce 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -112,14 +112,14 @@ def test_load_pipeline_config(self): def test_list_files_git(self): """Test listing pipeline files using `git ls`""" self.pipeline_obj._list_files() - assert os.path.join(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files + assert Path(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files @with_temporary_folder def test_list_files_no_git(self, tmpdir): """Test listing pipeline files without `git-ls`""" # Create a test file in a temporary directory - tmp_fn = os.path.join(tmpdir, "testfile") - Path(tmp_fn).touch() + tmp_fn = Path(tmpdir, "testfile") + tmp_fn.touch() pipeline_obj = nf_core.utils.Pipeline(tmpdir) pipeline_obj._list_files() assert tmp_fn in pipeline_obj.files From 030415e917a293add3d7dd6d733bc456a1869896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 26 Jan 2024 16:19:39 +0100 Subject: [PATCH 7/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/lint/merge_markers.py | 2 +- nf_core/lint/template_strings.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nf_core/lint/merge_markers.py b/nf_core/lint/merge_markers.py index a9b586a109..80cd655066 100644 --- a/nf_core/lint/merge_markers.py +++ b/nf_core/lint/merge_markers.py @@ -13,7 +13,7 @@ def merge_markers(self): This test looks for remaining merge markers in the code, e.g.: >>>>>>> or <<<<<<< - .. tip:: You can choose to ignore this test tests by creating a file called + .. tip:: You can choose to ignore this lint tests by editing the file called ``.nf-core.yml`` in the root of your pipeline and setting the test to false: .. code-block:: yaml lint: diff --git a/nf_core/lint/template_strings.py b/nf_core/lint/template_strings.py index 3e9cfd18fc..9b015bc209 100644 --- a/nf_core/lint/template_strings.py +++ b/nf_core/lint/template_strings.py @@ -17,7 +17,7 @@ def template_strings(self): This test ignores any double-brackets prefixed with a dollar sign, such as ``${{ secrets.AWS_ACCESS_KEY_ID }}`` as these placeholders are used in GitHub Actions workflows. - .. tip:: You can choose to ignore this test tests by creating a file called + .. tip:: You can choose to ignore lint test tests by editing the file called ``.nf-core.yml`` in the root of your pipeline and setting the test to false: .. code-block:: yaml @@ -26,7 +26,7 @@ def template_strings(self): template_strings: False To disable this test only for specific files, you can specify a list of file paths to ignore. - For exampke, to ignore a pdf you added to the docs: + For example, to ignore a pdf you added to the docs: .. code-block:: yaml