Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to update modules with linting #1588

Merged
merged 38 commits into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c5c1901
don't updatate sha if the module is not updated
Apr 19, 2022
f26c2d4
modify changelog
Apr 20, 2022
eb972a7
Merge branch 'dev' into update_modules
mirpedrol Apr 20, 2022
f3058b4
Merge branch 'dev' into update_modules
ggabernet May 5, 2022
2374756
don't updatate sha if the module is not updated
Apr 19, 2022
5b04b2b
resolve conflicts in changelog
Apr 20, 2022
e6b1af5
resolve conflicts in main_nf.py
mirpedrol May 16, 2022
cee29f0
fix conflicts
mirpedrol May 17, 2022
21d9a54
get version and build
mirpedrol May 17, 2022
db7fac1
check that url exist & add log info
mirpedrol May 17, 2022
72c49c9
improve logging
mirpedrol May 17, 2022
008350e
improve comments
mirpedrol May 17, 2022
4c87e9d
fix conflicts
mirpedrol May 17, 2022
e152b7b
remove changes made by error
mirpedrol May 17, 2022
52a60ab
merge branch
mirpedrol May 17, 2022
106b60e
update changelog
mirpedrol May 17, 2022
9d839cd
Merge branch 'dev' into update_modules
mirpedrol May 17, 2022
c1c4541
small changelog change
mirpedrol May 17, 2022
e6525ef
improve debug messages
mirpedrol May 19, 2022
faef8ea
Merge branch 'dev' into update_modules
mirpedrol May 30, 2022
0122056
don't updatate sha if the module is not updated
Apr 19, 2022
b288578
resolve conflicts in changelog
Apr 20, 2022
7b1538e
resolve conflicts in main_nf.py
mirpedrol May 16, 2022
ef00ede
fix conflicts
mirpedrol May 17, 2022
f205b15
get version and build
mirpedrol May 17, 2022
877b6f6
resolve rebase conflicts
mirpedrol Jun 2, 2022
3886ad1
improve logging
mirpedrol May 17, 2022
eac514d
improve comments
mirpedrol May 17, 2022
a5ba612
remove changes made by error
mirpedrol May 17, 2022
08e1b94
don't updatate sha if the module is not updated
Apr 19, 2022
99bf430
resolve rebase conflicts
mirpedrol Jun 2, 2022
550f6d2
improve debug messages
mirpedrol May 19, 2022
af8e030
resolve conflicts
mirpedrol Jun 2, 2022
3e9076c
refactor fix flag to fix-version
mirpedrol Jun 2, 2022
8e061f1
fix typo in CHANGELOG.md
mirpedrol Jun 2, 2022
0037bf9
Remove duplicate in CHANGELOG.md
mirpedrol Jun 2, 2022
cfcf3b6
Remove changed made by mistake
mirpedrol Jun 2, 2022
358bb0a
fix black linting
mirpedrol Jun 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
### Linting

- Check that the `.prettierignore` file exists and that starts with the same content.
- Add isort configuration and GitHub workflow ([#1538](https://github.com/nf-core/tools/pull/1538))
- Add `fix` flag to `nf-core modules lint` command to update modules to the latest version ([#1588](https://github.com/nf-core/tools/pull/1588))
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved

### General

Expand All @@ -18,12 +20,9 @@

### Modules

- Add `--fix-version` flag to `nf-core modules lint` command to update modules to the latest version ([#1588](https://github.com/nf-core/tools/pull/1588))
- Fix a bug in the regex extracting the version from biocontainers URLs [#1598](https://github.com/nf-core/tools/pull/1598)

### Linting

- Add isort configuration and GitHub workflow ([#1538](https://github.com/nf-core/tools/pull/1538))

## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16]

- Patch release to try to fix the template sync ([#1585](https://github.com/nf-core/tools/pull/1585))
Expand Down
13 changes: 11 additions & 2 deletions nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,8 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
@click.option("-a", "--all", is_flag=True, help="Run on all modules")
@click.option("--local", is_flag=True, help="Run additional lint tests for local modules")
@click.option("--passed", is_flag=True, help="Show passed tests")
def lint(ctx, tool, dir, key, all, local, passed):
@click.option("--fix-version", is_flag=True, help="Fix the module version if a newer version is available")
def lint(ctx, tool, dir, key, all, local, passed, fix_version):
"""
Lint one or more modules in a directory.

Expand All @@ -609,7 +610,15 @@ def lint(ctx, tool, dir, key, all, local, passed):
try:
module_lint = nf_core.modules.ModuleLint(dir=dir)
module_lint.modules_repo = ctx.obj["modules_repo_obj"]
module_lint.lint(module=tool, key=key, all_modules=all, print_results=True, local=local, show_passed=passed)
module_lint.lint(
module=tool,
key=key,
all_modules=all,
print_results=True,
local=local,
show_passed=passed,
fix_version=fix_version,
)
if len(module_lint.failed) > 0:
sys.exit(1)
except nf_core.modules.lint.ModuleLintException as e:
Expand Down
31 changes: 23 additions & 8 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,16 @@ def __init__(self, dir):
def _get_all_lint_tests():
return ["main_nf", "meta_yml", "module_todos", "module_deprecations"]

def lint(self, module=None, key=(), all_modules=False, print_results=True, show_passed=False, local=False):
def lint(
self,
module=None,
key=(),
all_modules=False,
print_results=True,
show_passed=False,
local=False,
fix_version=False,
):
"""
Lint all or one specific module

Expand All @@ -118,6 +127,7 @@ def lint(self, module=None, key=(), all_modules=False, print_results=True, show_
:param module: A specific module to lint
:param print_results: Whether to print the linting results
:param show_passed: Whether passed tests should be shown as well
:param fix_version: Update the module version if a newer version is available

:returns: A ModuleLint object containing information of
the passed, warned and failed tests
Expand Down Expand Up @@ -174,11 +184,11 @@ def lint(self, module=None, key=(), all_modules=False, print_results=True, show_

# Lint local modules
if local and len(local_modules) > 0:
self.lint_modules(local_modules, local=True)
self.lint_modules(local_modules, local=True, fix_version=fix_version)

# Lint nf-core modules
if len(nfcore_modules) > 0:
self.lint_modules(nfcore_modules, local=False)
self.lint_modules(nfcore_modules, local=False, fix_version=fix_version)

if print_results:
self._print_results(show_passed=show_passed)
Expand Down Expand Up @@ -282,19 +292,21 @@ def get_installed_modules(self):

return local_modules, nfcore_modules

def lint_modules(self, modules, local=False):
def lint_modules(self, modules, local=False, fix_version=False):
"""
Lint a list of modules

Args:
modules ([NFCoreModule]): A list of module objects
local (boolean): Whether the list consist of local or nf-core modules
fix_version (boolean): Fix the module version if a newer version is available
"""
progress_bar = rich.progress.Progress(
"[bold blue]{task.description}",
rich.progress.BarColumn(bar_width=None),
"[magenta]{task.completed} of {task.total}[reset] » [bold yellow]{task.fields[test_name]}",
transient=True,
console=console,
)
with progress_bar:
lint_progress = progress_bar.add_task(
Expand All @@ -305,9 +317,9 @@ def lint_modules(self, modules, local=False):

for mod in modules:
progress_bar.update(lint_progress, advance=1, test_name=mod.module_name)
self.lint_module(mod, local=local)
self.lint_module(mod, progress_bar, local=local, fix_version=fix_version)

def lint_module(self, mod, local=False):
def lint_module(self, mod, progress_bar, local=False, fix_version=False):
"""
Perform linting on one module

Expand All @@ -326,14 +338,17 @@ def lint_module(self, mod, local=False):

# Only check the main script in case of a local module
if local:
self.main_nf(mod)
self.main_nf(mod, fix_version, progress_bar)
self.passed += [LintResult(mod, *m) for m in mod.passed]
self.warned += [LintResult(mod, *m) for m in (mod.warned + mod.failed)]

# Otherwise run all the lint tests
else:
for test_name in self.lint_tests:
getattr(self, test_name)(mod)
if test_name == "main_nf":
getattr(self, test_name)(mod, fix_version, progress_bar)
else:
getattr(self, test_name)(mod)

self.passed += [LintResult(mod, *m) for m in mod.passed]
self.warned += [LintResult(mod, *m) for m in mod.warned]
Expand Down
113 changes: 104 additions & 9 deletions nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,19 @@
Lint the main.nf file of a module
"""

import logging
import re

import requests
from galaxy.tool_util.deps.mulled.util import build_target

import nf_core
import nf_core.modules.module_utils

log = logging.getLogger(__name__)


def main_nf(module_lint_object, module):
def main_nf(module_lint_object, module, fix_version, progress_bar):
"""
Lint a ``main.nf`` module file

Expand Down Expand Up @@ -100,7 +107,7 @@ def main_nf(module_lint_object, module):
module.passed.append(("main_nf_script_outputs", "Process 'output' block found", module.main_nf))

# Check the process definitions
if check_process_section(module, process_lines):
if check_process_section(module, process_lines, fix_version, progress_bar):
module.passed.append(("main_nf_container", "Container versions match", module.main_nf))
else:
module.warned.append(("main_nf_container", "Container versions do not match", module.main_nf))
Expand Down Expand Up @@ -190,7 +197,7 @@ def check_when_section(self, lines):
self.passed.append(("when_condition", "when: condition is unchanged", self.main_nf))


def check_process_section(self, lines):
def check_process_section(self, lines, fix_version, progress_bar):
"""
Lint the section of a module between the process definition
and the 'input:' definition
Expand Down Expand Up @@ -236,14 +243,14 @@ def check_process_section(self, lines):
self.warned.append(("process_standard_label", "Process label unspecified", self.main_nf))

for l in lines:
if re.search("bioconda::", l):
if _container_type(l) == "bioconda":
bioconda_packages = [b for b in l.split() if "bioconda::" in b]
l = l.strip(" '\"")
if l.startswith("https://containers") or l.startswith("https://depot"):
if _container_type(l) == "singularity":
# e.g. "https://containers.biocontainers.pro/s3/SingImgsRepo/biocontainers/v1.2.0_cv1/biocontainers_v1.2.0_cv1.img' :" -> v1.2.0_cv1
# e.g. "https://depot.galaxyproject.org/singularity/fastqc:0.11.9--0' :" -> 0.11.9--0
singularity_tag = re.search(r"(?:/)?(?:biocontainers_)?(?::)?([A-Za-z\d\-_.]+?)(?:\.img)?['\"]", l).group(1)
if l.startswith("biocontainers/") or l.startswith("quay.io/"):
if _container_type(l) == "docker":
# e.g. "quay.io/biocontainers/krona:2.7.1--pl526_5' }" -> 2.7.1--pl526_5
# e.g. "biocontainers/biocontainers:v1.2.0_cv1' }" -> v1.2.0_cv1
docker_tag = re.search(r"(?:[/])?(?::)?([A-Za-z\d\-_.]+)['\"]", l).group(1)
Expand Down Expand Up @@ -272,9 +279,30 @@ def check_process_section(self, lines):
last_ver = response.get("latest_version")
if last_ver is not None and last_ver != bioconda_version:
package, ver = bp.split("=", 1)
self.warned.append(
("bioconda_latest", f"Conda update: {package} `{ver}` -> `{last_ver}`", self.main_nf)
)
# If a new version is available and fix is True, update the version
if fix_version:
if _fix_module_version(self, bioconda_version, last_ver, singularity_tag, response):
progress_bar.print(f"[blue]INFO[/blue]\t Updating package '{package}' {ver} -> {last_ver}")
log.debug(f"Updating package {package} {ver} -> {last_ver}")
self.passed.append(
(
"bioconda_latest",
f"Conda package has been updated to the latest available: `{bp}`",
self.main_nf,
)
)
else:
progress_bar.print(
f"[blue]INFO[/blue]\t Tried to update package. Unable to update package '{package}' {ver} -> {last_ver}"
)
log.debug(f"Unable to update package {package} {ver} -> {last_ver}")
self.warned.append(
("bioconda_latest", f"Conda update: {package} `{ver}` -> `{last_ver}`", self.main_nf)
)
else:
self.warned.append(
("bioconda_latest", f"Conda update: {package} `{ver}` -> `{last_ver}`", self.main_nf)
)
else:
self.passed.append(("bioconda_latest", f"Conda package is the latest available: `{bp}`", self.main_nf))

Expand Down Expand Up @@ -341,3 +369,70 @@ def _is_empty(self, line):
if line.strip().replace(" ", "") == "":
empty = True
return empty


def _fix_module_version(self, current_version, latest_version, singularity_tag, response):
"""Updates the module version

Changes the bioconda current version by the latest version.
Obtains the latest build from bioconda response
Checks that the new URLs for docker and singularity with the tag [version]--[build] are valid
Changes the docker and singularity URLs
"""
# Get latest build
build = _get_build(response)

with open(self.main_nf, "r") as source:
lines = source.readlines()

# Check if the new version + build exist and replace
new_lines = []
for line in lines:
l = line.strip(" '\"")
build_type = _container_type(l)
if build_type == "bioconda":
new_lines.append(re.sub(rf"{current_version}", f"{latest_version}", line))
elif build_type == "singularity" or build_type == "docker":
# Check that the new url is valid
new_url = re.search(
"(?:')(.+)(?:')", re.sub(rf"{singularity_tag}", f"{latest_version}--{build}", line)
).group(1)
response_new_container = requests.get(
"https://" + new_url if not new_url.startswith("https://") else new_url, stream=True
)
log.debug(
f"Connected to URL: {'https://' + new_url if not new_url.startswith('https://') else new_url}, status_code: {response_new_container.status_code}"
)
if response_new_container.status_code != 200:
return False
new_lines.append(re.sub(rf"{singularity_tag}", f"{latest_version}--{build}", line))
else:
new_lines.append(line)

# Replace outdated versions by the latest one
with open(self.main_nf, "w") as source:
for line in new_lines:
source.write(line)

return True


def _get_build(response):
"""Get the latest build of the container version"""
build_times = []
latest_v = response.get("latest_version")
files = response.get("files")
for f in files:
if f.get("version") == latest_v:
build_times.append((f.get("upload_time"), f.get("attrs").get("build")))
return sorted(build_times, key=lambda tup: tup[0], reverse=True)[0][1]


def _container_type(line):
"""Returns the container type of a build."""
if re.search("bioconda::", line):
return "bioconda"
if line.startswith("https://containers") or line.startswith("https://depot"):
return "singularity"
if line.startswith("biocontainers/") or line.startswith("quay.io/"):
return "docker"
6 changes: 0 additions & 6 deletions nf_core/modules/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,6 @@ class DiffEnum(enum.Enum):
if not dry_run:
self.update_modules_json(modules_json, modules_repo.name, module, version)

mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
# Don't save to a file, just iteratively update the variable
else:
modules_json = self.update_modules_json(
modules_json, modules_repo.name, module, version, write_file=False
)

if self.save_diff_fn:
# Compare the new modules.json and build a diff
modules_json_diff = difflib.unified_diff(
Expand Down