diff --git a/.github/workflows/branch.yml b/.github/workflows/branch.yml
index 5437eeba8b..1f3d241d5f 100644
--- a/.github/workflows/branch.yml
+++ b/.github/workflows/branch.yml
@@ -21,11 +21,19 @@ jobs:
uses: mshick/add-pr-comment@v1
with:
message: |
+ ## This PR is against the `master` branch :x:
+
+ * Do not close this PR
+ * Click _Edit_ and change the `base` to `dev`
+ * This CI test will remain failed until you push a new commit
+
+ ---
+
Hi @${{ github.event.pull_request.user.login }},
- It looks like this pull-request has been made against the ${{github.event.pull_request.base.repo.full_name}} `master` branch.
+ It looks like this pull-request is has been made against the [${{github.event.pull_request.head.repo.full_name }}](https://github.com/${{github.event.pull_request.head.repo.full_name }}) `master` branch.
The `master` branch on nf-core repositories should always contain code from the latest release.
- Because of this, PRs to `master` are only allowed if they come from the ${{github.event.pull_request.base.repo.full_name}} `dev` branch.
+ Because of this, PRs to `master` are only allowed if they come from the [${{github.event.pull_request.head.repo.full_name }}](https://github.com/${{github.event.pull_request.head.repo.full_name }}) `dev` branch.
You do not need to close this PR, you can change the target branch to `dev` by clicking the _"Edit"_ button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 60fce6a0cd..7e27d8ca0a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
# nf-core/tools: Changelog
+## [v1.13.1 - Copper Crocodile Patch :crocodile: :pirate_flag:](https://github.com/nf-core/tools/releases/tag/1.13.1) - [2021-03-19]
+
+* Fixed bug in pipeline linting markdown output that gets posted to PR comments [[#914]](https://github.com/nf-core/tools/issues/914)
+* Made text for the PR branch CI check less verbose with a TLDR in bold at the top
+* A number of minor tweaks to the new `nf-core modules lint` code
+
## [v1.13 - Copper Crocodile](https://github.com/nf-core/tools/releases/tag/1.13) - [2021-03-18]
### Template
diff --git a/docs/images/nfcore-tools_logo.png b/docs/images/nfcore-tools_logo.png
old mode 100755
new mode 100644
index 9a0659d9d1..e45813591e
Binary files a/docs/images/nfcore-tools_logo.png and b/docs/images/nfcore-tools_logo.png differ
diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py
index b52d1dd230..338c908bbb 100644
--- a/nf_core/lint/__init__.py
+++ b/nf_core/lint/__init__.py
@@ -16,7 +16,6 @@
import re
import rich
import rich.progress
-import textwrap
import yaml
import nf_core.utils
@@ -348,6 +347,8 @@ def _get_results_md(self):
"""
# Overall header
overall_result = "Passed :white_check_mark:"
+ if len(self.warned) > 0:
+ overall_result += " :warning:"
if len(self.failed) > 0:
overall_result = "Failed :x:"
@@ -431,24 +432,16 @@ def _get_results_md(self):
comment_body_text = "Posted for pipeline commit {}".format(self.git_sha[:7]) if self.git_sha is not None else ""
timestamp = now.strftime("%Y-%m-%d %H:%M:%S")
- markdown = textwrap.dedent(
- f"""
- #### `nf-core lint` overall result: {overall_result}
-
- {comment_body_text}
-
- ```diff{test_passed_count}{test_ignored_count}{test_fixed_count}{test_warning_count}{test_failure_count}
- ```
-
-
-
- {test_failures}{test_warnings}{test_ignored}{test_fixed}{test_passes}### Run details:
-
- * nf-core/tools version {nf_core.__version__}
- * Run at `{timestamp}`
-
-
- """
+ markdown = (
+ f"## `nf-core lint` overall result: {overall_result}\n\n"
+ f"{comment_body_text}\n\n"
+ f"```diff{test_passed_count}{test_ignored_count}{test_fixed_count}{test_warning_count}{test_failure_count}\n"
+ "```\n\n"
+ "\n"
+ f"{test_failures}{test_warnings}{test_ignored}{test_fixed}{test_passes}### Run details\n\n"
+ f"* nf-core/tools version {nf_core.__version__}\n"
+ f"* Run at `{timestamp}`\n\n"
+ " \n"
)
return markdown
diff --git a/nf_core/modules/lint.py b/nf_core/modules/lint.py
index 00ef496e9d..909dd91170 100644
--- a/nf_core/modules/lint.py
+++ b/nf_core/modules/lint.py
@@ -9,6 +9,7 @@
from __future__ import print_function
import logging
+import operator
import os
import questionary
import re
@@ -34,6 +35,17 @@ class ModuleLintException(Exception):
pass
+class LintResult(object):
+ """ An object to hold the results of a lint test """
+
+ def __init__(self, mod, lint_test, message, file_path):
+ self.mod = mod
+ self.lint_test = lint_test
+ self.message = message
+ self.file_path = file_path
+ self.module_name = mod.module_name
+
+
class ModuleLint(object):
"""
An object for linting modules either in a clone of the 'nf-core/modules'
@@ -144,8 +156,8 @@ def lint_local_modules(self, local_modules):
mod_object.main_nf = mod
mod_object.module_name = os.path.basename(mod)
mod_object.lint_main_nf()
- self.passed = [(mod_object, m) for m in mod_object.passed]
- self.warned = [(mod_object, m) for m in mod_object.warned + mod_object.failed]
+ self.passed += [LintResult(mod_object, m[0], m[1], m[2]) for m in mod_object.passed]
+ self.warned += [LintResult(mod_object, m[0], m[1], m[2]) for m in mod_object.warned + mod_object.failed]
def lint_nfcore_modules(self, nfcore_modules):
"""
@@ -174,12 +186,9 @@ def lint_nfcore_modules(self, nfcore_modules):
for mod in nfcore_modules:
progress_bar.update(lint_progress, advance=1, test_name=mod.module_name)
passed, warned, failed = mod.lint()
- passed = [(mod, m) for m in passed]
- warned = [(mod, m) for m in warned]
- failed = [(mod, m) for m in failed]
- self.passed += passed
- self.warned += warned
- self.failed += failed
+ self.passed += [LintResult(mod, m[0], m[1], m[2]) for m in passed]
+ self.warned += [LintResult(mod, m[0], m[1], m[2]) for m in warned]
+ self.failed += [LintResult(mod, m[0], m[1], m[2]) for m in failed]
def get_repo_type(self):
"""
@@ -235,6 +244,10 @@ def get_installed_modules(self):
# Get nf-core modules
if os.path.exists(nfcore_modules_dir):
for m in sorted([m for m in os.listdir(nfcore_modules_dir) if not m == "lib"]):
+ if not os.path.isdir(os.path.join(nfcore_modules_dir, m)):
+ raise ModuleLintException(
+ f"File found in '{nfcore_modules_dir}': '{m}'! This directory should only contain module directories."
+ )
m_content = os.listdir(os.path.join(nfcore_modules_dir, m))
# Not a module, but contains sub-modules
if not "main.nf" in m_content:
@@ -262,12 +275,17 @@ def _print_results(self, show_passed=False):
log.debug("Printing final results")
console = Console(force_terminal=rich_force_colors())
+ # Sort the results
+ self.passed.sort(key=operator.attrgetter("message", "module_name"))
+ self.warned.sort(key=operator.attrgetter("message", "module_name"))
+ self.failed.sort(key=operator.attrgetter("message", "module_name"))
+
# Find maximum module name length
max_mod_name_len = 40
for idx, tests in enumerate([self.passed, self.warned, self.failed]):
try:
- for mod, msg in tests:
- max_mod_name_len = max(len(mod.module_name), max_mod_name_len)
+ for lint_result in tests:
+ max_mod_name_len = max(len(lint_result.module_name), max_mod_name_len)
except:
pass
@@ -281,17 +299,17 @@ def format_result(test_results, table):
# I'd like to make an issue about this on the rich repo so leaving here in case there is a future fix
last_modname = False
row_style = None
- for mod, result in test_results:
- if last_modname and mod.module_name != last_modname:
+ for lint_result in test_results:
+ if last_modname and lint_result.module_name != last_modname:
if row_style:
row_style = None
else:
row_style = "magenta"
- last_modname = mod.module_name
+ last_modname = lint_result.module_name
table.add_row(
- Markdown(f"{mod.module_name}"),
- Markdown(f"{result[1]}"),
- os.path.relpath(result[2], self.dir),
+ Markdown(f"{lint_result.module_name}"),
+ os.path.relpath(lint_result.file_path, self.dir),
+ Markdown(f"{lint_result.message}"),
style=row_style,
)
return table
@@ -308,8 +326,8 @@ def _s(some_list):
)
table = Table(style="green", box=rich.box.ROUNDED)
table.add_column("Module name", width=max_mod_name_len)
- table.add_column("Test message", no_wrap=True)
- table.add_column("File path", no_wrap=True)
+ table.add_column("File path")
+ table.add_column("Test message")
table = format_result(self.passed, table)
console.print(table)
@@ -322,8 +340,8 @@ def _s(some_list):
)
table = Table(style="yellow", box=rich.box.ROUNDED)
table.add_column("Module name", width=max_mod_name_len)
- table.add_column("Test message", no_wrap=True)
- table.add_column("File path", no_wrap=True)
+ table.add_column("File path")
+ table.add_column("Test message")
table = format_result(self.warned, table)
console.print(table)
@@ -334,8 +352,8 @@ def _s(some_list):
)
table = Table(style="red", box=rich.box.ROUNDED)
table.add_column("Module name", width=max_mod_name_len)
- table.add_column("Test message", no_wrap=True)
- table.add_column("File path", no_wrap=True)
+ table.add_column("File path")
+ table.add_column("Test message")
table = format_result(self.failed, table)
console.print(table)
@@ -390,13 +408,11 @@ def check_module_changes(self, nfcore_modules):
if r.status_code != 200:
self.warned.append(
- (
+ LintResult(
mod,
- (
- "check_local_copy",
- f"Could not fetch remote copy, skipping comparison.",
- f"{os.path.join(mod.module_dir, f)}",
- ),
+ "check_local_copy",
+ f"Could not fetch remote copy, skipping comparison.",
+ f"{os.path.join(mod.module_dir, f)}",
)
)
else:
@@ -406,24 +422,20 @@ def check_module_changes(self, nfcore_modules):
if local_copy != remote_copy:
all_modules_up_to_date = False
self.warned.append(
- (
+ LintResult(
mod,
- (
- "check_local_copy",
- "Local copy of module outdated",
- f"{os.path.join(mod.module_dir, f)}",
- ),
+ "check_local_copy",
+ "Local copy of module outdated",
+ f"{os.path.join(mod.module_dir, f)}",
)
)
except UnicodeDecodeError as e:
self.warned.append(
- (
+ LintResult(
mod,
- (
- "check_local_copy",
- f"Could not decode file from {url}. Skipping comparison ({e})",
- f"{os.path.join(mod.module_dir, f)}",
- ),
+ "check_local_copy",
+ f"Could not decode file from {url}. Skipping comparison ({e})",
+ f"{os.path.join(mod.module_dir, f)}",
)
)
@@ -510,7 +522,8 @@ def lint_module_tests(self):
def lint_meta_yml(self):
""" Lint a meta yml file """
- required_keys = ["input", "output"]
+ required_keys = ["name", "input", "output"]
+ required_keys_lists = ["intput", "output"]
try:
with open(self.meta_yml, "r") as fh:
meta_yaml = yaml.safe_load(fh)
@@ -526,7 +539,7 @@ def lint_meta_yml(self):
if not rk in meta_yaml.keys():
self.failed.append(("meta_required_keys", f"`{rk}` not specified", self.meta_yml))
contains_required_keys = False
- elif not isinstance(meta_yaml[rk], list):
+ elif not isinstance(meta_yaml[rk], list) and rk in required_keys_lists:
self.failed.append(("meta_required_keys", f"`{rk}` is not a list", self.meta_yml))
all_list_children = False
if contains_required_keys:
@@ -548,13 +561,13 @@ def lint_meta_yml(self):
else:
self.failed.append(("meta_output", "`{output}` missing in `meta.yml`", self.meta_yml))
- # confirm that the name matches the process name in main.nf
- if meta_yaml["name"].upper() == self.process_name:
- self.passed.append(("meta_name", "Correct name specified in `meta.yml`", self.meta_yml))
- else:
- self.failed.append(
- ("meta_name", "Conflicting process name between `meta.yml` and `main.nf`", self.meta_yml)
- )
+ # confirm that the name matches the process name in main.nf
+ if meta_yaml["name"].upper() == self.process_name:
+ self.passed.append(("meta_name", "Correct name specified in `meta.yml`", self.meta_yml))
+ else:
+ self.failed.append(
+ ("meta_name", "Conflicting process name between `meta.yml` and `main.nf`", self.meta_yml)
+ )
def lint_main_nf(self):
"""
@@ -722,9 +735,9 @@ def check_process_section(self, lines):
# response = _bioconda_package(bp)
response = nf_core.utils.anaconda_package(bp)
except LookupError as e:
- self.warned.append(e)
+ self.warned.append(("bioconda_version", "Conda version not specified correctly", self.main_nf))
except ValueError as e:
- self.failed.append(e)
+ self.failed.append(("bioconda_version", "Conda version not specified correctly", self.main_nf))
else:
# Check that required version is available at all
if bioconda_version not in response.get("versions"):
@@ -794,7 +807,8 @@ def _parse_output(self, line):
output = []
if "meta" in line:
output.append("meta")
- # TODO: should we ignore outputs without emit statement?
+ if not "emit" in line:
+ self.failed.append(("missing_emit", f"Missing emit statement: {line.strip()}", self.main_nf))
if "emit" in line:
output.append(line.split("emit:")[1].strip())
diff --git a/nf_core/pipeline-template/.github/workflows/branch.yml b/nf_core/pipeline-template/.github/workflows/branch.yml
index 5c880e7c5f..281d1af157 100644
--- a/nf_core/pipeline-template/.github/workflows/branch.yml
+++ b/nf_core/pipeline-template/.github/workflows/branch.yml
@@ -23,11 +23,19 @@ jobs:
uses: mshick/add-pr-comment@v1
with:
message: |
+ ## This PR is against the `master` branch :x:
+
+ * Do not close this PR
+ * Click _Edit_ and change the `base` to `dev`
+ * This CI test will remain failed until you push a new commit
+
+ ---
+
Hi @${{ github.event.pull_request.user.login }},
- It looks like this pull-request is has been made against the ${{github.event.pull_request.base.repo.full_name }} `master` branch.
+ It looks like this pull-request is has been made against the [${{github.event.pull_request.head.repo.full_name }}](https://github.com/${{github.event.pull_request.head.repo.full_name }}) `master` branch.
The `master` branch on nf-core repositories should always contain code from the latest release.
- Because of this, PRs to `master` are only allowed if they come from the ${{github.event.pull_request.base.repo.full_name }} `dev` branch.
+ Because of this, PRs to `master` are only allowed if they come from the [${{github.event.pull_request.head.repo.full_name }}](https://github.com/${{github.event.pull_request.head.repo.full_name }}) `dev` branch.
You do not need to close this PR, you can change the target branch to `dev` by clicking the _"Edit"_ button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.
diff --git a/setup.py b/setup.py
index b9b40abc8a..280d21559a 100644
--- a/setup.py
+++ b/setup.py
@@ -3,7 +3,7 @@
from setuptools import setup, find_packages
import sys
-version = "1.13"
+version = "1.13.1"
with open("README.md") as f:
readme = f.read()