Skip to content

Commit

Permalink
Merge pull request #2539 from mashehu/test-modules-bump
Browse files Browse the repository at this point in the history
improve testing coverage for meta_yml
  • Loading branch information
mashehu authored Nov 27, 2023
2 parents 8b4addf + 341dbdf commit db3c449
Show file tree
Hide file tree
Showing 11 changed files with 319 additions and 83 deletions.
61 changes: 32 additions & 29 deletions nf_core/components/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
from typing import Dict, List, Optional, Tuple, Union, cast

import rich
import rich.table

from nf_core.components.components_command import ComponentCommand
from nf_core.modules.modules_json import ModulesJson
Expand All @@ -24,7 +24,7 @@ def __init__(
super().__init__(component_type, pipeline_dir, remote_url, branch, no_pull)
self.remote = remote

def list_components(self, keywords: Optional[List[str]] = None, print_json=False) -> rich.table.Table:
def list_components(self, keywords: Optional[List[str]] = None, print_json=False) -> Union[rich.table.Table, str]:
keywords = keywords or []
"""
Get available modules/subworkflows names from GitHub tree for repo
Expand All @@ -38,7 +38,7 @@ def list_components(self, keywords: Optional[List[str]] = None, print_json=False
table.add_column(f"{self.component_type[:-1].capitalize()} Name")
components: List[str] = []

def pattern_msg(keywords: List[str]):
def pattern_msg(keywords: List[str]) -> str:
if len(keywords) == 0:
return ""
if len(keywords) == 1:
Expand Down Expand Up @@ -107,37 +107,40 @@ def pattern_msg(keywords: List[str]):
table.add_column("Date")

# Load 'modules.json'
modules_json = modules_json.modules_json
modules_json_file = modules_json.modules_json

for repo_url, component_with_dir in sorted(repos_with_comps.items()):
repo_entry: Dict[str, Dict[str, Dict[str, Dict[str, Union[str, List[str]]]]]]

repo_entry = modules_json["repos"].get(repo_url, {})
for install_dir, component in sorted(component_with_dir):
# Use cast() to predict the return type of recursive get():s
repo_modules = cast(dict, repo_entry.get(self.component_type))
component_entry = cast(dict, cast(dict, repo_modules.get(install_dir)).get(component))

if component_entry:
version_sha = component_entry["git_sha"]
try:
# pass repo_name to get info on modules even outside nf-core/modules
message, date = ModulesRepo(
remote_url=repo_url,
branch=component_entry["branch"],
).get_commit_info(version_sha)
except LookupError as e:
log.warning(e)
if modules_json_file is None:
log.warning(f"Modules JSON file '{modules_json.modules_json_path}' is missing. ")
continue
else:
repo_entry = modules_json_file["repos"].get(repo_url, {})
for install_dir, component in sorted(component_with_dir):
# Use cast() to predict the return type of recursive get():s
repo_modules = cast(dict, repo_entry.get(self.component_type))
component_entry = cast(dict, cast(dict, repo_modules.get(install_dir)).get(component))

if component_entry:
version_sha = component_entry["git_sha"]
try:
# pass repo_name to get info on modules even outside nf-core/modules
message, date = ModulesRepo(
remote_url=repo_url,
branch=component_entry["branch"],
).get_commit_info(version_sha)
except LookupError as e:
log.warning(e)
date = "[red]Not Available"
message = "[red]Not Available"
else:
log.warning(
f"Commit SHA for {self.component_type[:-1]} '{install_dir}/{self.component_type}' is missing from 'modules.json'"
)
version_sha = "[red]Not Available"
date = "[red]Not Available"
message = "[red]Not Available"
else:
log.warning(
f"Commit SHA for {self.component_type[:-1]} '{install_dir}/{self.component_type}' is missing from 'modules.json'"
)
version_sha = "[red]Not Available"
date = "[red]Not Available"
message = "[red]Not Available"
table.add_row(component, repo_url, version_sha, message, date)
table.add_row(component, repo_url, version_sha, message, date)

if print_json:
return json.dumps(components, sort_keys=True, indent=4)
Expand Down
60 changes: 54 additions & 6 deletions nf_core/components/nfcore_component.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
"""
The NFCoreComponent class holds information and utility functions for a single module or subworkflow
"""
import logging
import re
from pathlib import Path

log = logging.getLogger(__name__)


class NFCoreComponent:
"""
Expand Down Expand Up @@ -44,16 +48,16 @@ def __init__(

if remote_component:
# Initialize the important files
self.main_nf = self.component_dir / "main.nf"
self.meta_yml = self.component_dir / "meta.yml"
self.main_nf = Path(self.component_dir, "main.nf")
self.meta_yml = Path(self.component_dir, "meta.yml")
self.process_name = ""
self.environment_yml = self.component_dir / "environment.yml"
self.environment_yml = Path(self.component_dir, "environment.yml")

repo_dir = self.component_dir.parts[: self.component_dir.parts.index(self.component_name.split("/")[0])][-1]
self.org = repo_dir
self.nftest_testdir = self.component_dir / "tests"
self.nftest_main_nf = self.nftest_testdir / "main.nf.test"
self.tags_yml = self.nftest_testdir / "tags.yml"
self.nftest_testdir = Path(self.component_dir, "tests")
self.nftest_main_nf = Path(self.nftest_testdir, "main.nf.test")
self.tags_yml = Path(self.nftest_testdir, "tags.yml")

if self.repo_type == "pipeline":
patch_fn = f"{self.component_name.replace('/', '-')}.diff"
Expand Down Expand Up @@ -90,3 +94,47 @@ def _get_included_components(self, main_nf: str):
if line.strip().startswith("include"):
included_components.append(line.strip().split()[-1].split(self.org)[-1].split("main")[0].strip("/"))
return included_components

def get_inputs_from_main_nf(self):
"""Collect all inputs from the main.nf file."""
inputs = []
with open(self.main_nf, "r") as f:
data = f.read()
# get input values from main.nf after "input:", which can be formatted as tuple val(foo) path(bar) or val foo or val bar or path bar or path foo
# regex matches:
# val(foo)
# path(bar)
# val foo
# val bar
# path bar
# path foo
# don't match anything inside comments or after "output:"
if "input:" not in data:
log.info(f"Could not find any inputs in {self.main_nf}")
return inputs
input_data = data.split("input:")[1].split("output:")[0]
regex = r"(val|path)\s*(\(([^)]+)\)|\s*([^)\s,]+))"
matches = re.finditer(regex, input_data, re.MULTILINE)
for matchNum, match in enumerate(matches, start=1):
if match.group(3):
inputs.append(match.group(3))
elif match.group(4):
inputs.append(match.group(4))
log.info(f"Found {len(inputs)} inputs in {self.main_nf}")
self.inputs = inputs

def get_outputs_from_main_nf(self):
outputs = []
with open(self.main_nf, "r") as f:
data = f.read()
# get output values from main.nf after "output:". the names are always after "emit:"
if "output:" not in data:
log.info(f"Could not find any outputs in {self.main_nf}")
return outputs
output_data = data.split("output:")[1].split("when:")[0]
regex = r"emit:\s*([^)\s,]+)"
matches = re.finditer(regex, output_data, re.MULTILINE)
for matchNum, match in enumerate(matches, start=1):
outputs.append(match.group(1))
log.info(f"Found {len(outputs)} outputs in {self.main_nf}")
self.outputs = outputs
2 changes: 1 addition & 1 deletion nf_core/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import time
from pathlib import Path

import filetype
import filetype # type: ignore
import git
import jinja2
import questionary
Expand Down
44 changes: 24 additions & 20 deletions nf_core/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,30 @@ class PipelineLint(nf_core.utils.Pipeline):
warned (list): A list of tuples of the form: ``(<warned no>, <reason>)``
"""

from .actions_awsfulltest import actions_awsfulltest
from .actions_awstest import actions_awstest
from .actions_ci import actions_ci
from .actions_schema_validation import actions_schema_validation
from .files_exist import files_exist
from .files_unchanged import files_unchanged
from .merge_markers import merge_markers
from .modules_json import modules_json
from .modules_structure import modules_structure
from .multiqc_config import multiqc_config
from .nextflow_config import nextflow_config
from .pipeline_name_conventions import pipeline_name_conventions
from .pipeline_todos import pipeline_todos
from .readme import readme
from .schema_description import schema_description
from .schema_lint import schema_lint
from .schema_params import schema_params
from .system_exit import system_exit
from .template_strings import template_strings
from .version_consistency import version_consistency
from .actions_awsfulltest import actions_awsfulltest # type: ignore[misc]
from .actions_awstest import actions_awstest # type: ignore[misc]
from .actions_ci import actions_ci # type: ignore[misc]
from .actions_schema_validation import ( # type: ignore[misc]
actions_schema_validation,
)
from .files_exist import files_exist # type: ignore[misc]
from .files_unchanged import files_unchanged # type: ignore[misc]
from .merge_markers import merge_markers # type: ignore[misc]
from .modules_json import modules_json # type: ignore[misc]
from .modules_structure import modules_structure # type: ignore[misc]
from .multiqc_config import multiqc_config # type: ignore[misc]
from .nextflow_config import nextflow_config # type: ignore[misc]
from .pipeline_name_conventions import ( # type: ignore[misc]
pipeline_name_conventions,
)
from .pipeline_todos import pipeline_todos # type: ignore[misc]
from .readme import readme # type: ignore[misc]
from .schema_description import schema_description # type: ignore[misc]
from .schema_lint import schema_lint # type: ignore[misc]
from .schema_params import schema_params # type: ignore[misc]
from .system_exit import system_exit # type: ignore[misc]
from .template_strings import template_strings # type: ignore[misc]
from .version_consistency import version_consistency # type: ignore[misc]

def __init__(
self, wf_path, release_mode=False, fix=(), key=None, fail_ignored=False, fail_warned=False, hide_progress=False
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/bump_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def __init__(
self.tools_config: Dict[str, Any] = {}

def bump_versions(
self, module: Union[NFCoreComponent, None] = None, all_modules: bool = False, show_uptodate: bool = False
self, module: Union[str, None] = None, all_modules: bool = False, show_uptodate: bool = False
) -> None:
"""
Bump the container and conda version of single module or all modules
Expand Down
90 changes: 80 additions & 10 deletions nf_core/modules/lint/meta_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,24 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
and module input is consistent between the
``meta.yml`` and the ``main.nf``.
If the module has inputs or outputs, they are expected to be
formatted as:
..code-block::
tuple val(foo) path(bar)
val foo
path foo
or permutations of the above.
Args:
module_lint_object (ComponentLint): The lint object for the module
module (NFCoreComponent): The module to lint
"""

module.get_inputs_from_main_nf()
module.get_outputs_from_main_nf()
# Check if we have a patch file, get original file in that case
meta_yaml = None
if module.is_patched:
Expand All @@ -45,14 +62,14 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
return

# Confirm that the meta.yml file is valid according to the JSON schema
valid_meta_yml = True
valid_meta_yml = False
try:
with open(Path(module_lint_object.modules_repo.local_repo_dir, "modules/meta-schema.json"), "r") as fh:
schema = json.load(fh)
validators.validate(instance=meta_yaml, schema=schema)
module.passed.append(("meta_yml_valid", "Module `meta.yml` is valid", module.meta_yml))
valid_meta_yml = True
except exceptions.ValidationError as e:
valid_meta_yml = False
hint = ""
if len(e.path) > 0:
hint = f"\nCheck the entry for `{e.path[0]}`."
Expand All @@ -79,26 +96,79 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
meta_input = [list(x.keys())[0] for x in meta_yaml["input"]]
for input in module.inputs:
if input in meta_input:
module.passed.append(("meta_input", f"`{input}` specified", module.meta_yml))
module.passed.append(("meta_input_main_only", f"`{input}` specified", module.meta_yml))
else:
module.warned.append(
(
"meta_input_main_only",
f"`{input}` is present as an input in the `main.nf`, but missing in `meta.yml`",
module.meta_yml,
)
)
# check if there are any inputs in meta.yml that are not in main.nf
for input in meta_input:
if input in module.inputs:
module.passed.append(
(
"meta_input_meta_only",
f"`{input}` is present as an input in `meta.yml` and `main.nf`",
module.meta_yml,
)
)
else:
module.failed.append(("meta_input", f"`{input}` missing in `meta.yml`", module.meta_yml))
module.warned.append(
(
"meta_input_meta_only",
f"`{input}` is present as an input in `meta.yml` but not in `main.nf`",
module.meta_yml,
)
)

if "output" in meta_yaml:
if "output" in meta_yaml and meta_yaml["output"] is not None:
meta_output = [list(x.keys())[0] for x in meta_yaml["output"]]
for output in module.outputs:
if output in meta_output:
module.passed.append(("meta_output", f"`{output}` specified", module.meta_yml))
module.passed.append(("meta_output_main_only", f"`{output}` specified", module.meta_yml))
else:
module.failed.append(("meta_output", f"`{output}` missing in `meta.yml`", module.meta_yml))

module.warned.append(
(
"meta_output_main_only",
f"`{output}` is present as an output in the `main.nf`, but missing in `meta.yml`",
module.meta_yml,
)
)
# check if there are any outputs in meta.yml that are not in main.nf
for output in meta_output:
if output in module.outputs:
module.passed.append(
(
"meta_output_meta_only",
f"`{output}` is present as an output in `meta.yml` and `main.nf`",
module.meta_yml,
)
)
else:
module.warned.append(
(
"meta_output_meta_only",
f"`{output}` is present as an output in `meta.yml` but not in `main.nf`",
module.meta_yml,
)
)
# confirm that the name matches the process name in main.nf
if meta_yaml["name"].upper() == module.process_name:
module.passed.append(("meta_name", "Correct name specified in `meta.yml`", module.meta_yml))
module.passed.append(
(
"meta_name",
"Correct name specified in `meta.yml`.",
module.meta_yml,
)
)
else:
module.failed.append(
(
"meta_name",
f"Conflicting process name between meta.yml (`{meta_yaml['name']}`) and main.nf (`{module.process_name}`)",
f"Conflicting `process` name between meta.yml (`{meta_yaml['name']}`) and main.nf (`{module.process_name}`)",
module.meta_yml,
)
)
Loading

0 comments on commit db3c449

Please sign in to comment.