Skip to content

Commit

Permalink
Merge pull request #922 from nf-core/dev
Browse files Browse the repository at this point in the history 
Dev > Master for 1.13.1 release
  • Loading branch information
@ewels
ewels authored Mar 19, 2021
2 parents 03ab820 + e5f53bf commit b86ab1f
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 76 deletions.
12 changes: 10 additions & 2 deletions .github/workflows/branch.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Binary file modified docs/images/nfcore-tools_logo.png
100755 → 100644
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 12 additions & 19 deletions nf_core/lint/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import re
import rich
import rich.progress
import textwrap
import yaml

import nf_core.utils
Expand Down Expand Up @@ -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:"

Expand Down Expand Up @@ -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}
```
<details>
{test_failures}{test_warnings}{test_ignored}{test_fixed}{test_passes}### Run details:
* nf-core/tools version {nf_core.__version__}
* Run at `{timestamp}`
</details>
"""
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"
"<details>\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"
"</details>\n"
)

return markdown
Expand Down
118 changes: 66 additions & 52 deletions nf_core/modules/lint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from __future__ import print_function
import logging
import operator
import os
import questionary
import re
Expand All @@ -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'
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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:
Expand All @@ -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)}",
)
)

Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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())

Expand Down
12 changes: 10 additions & 2 deletions nf_core/pipeline-template/.github/workflows/branch.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit b86ab1f

Please sign in to comment.