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 --migrate-pytest option #2549

Merged
merged 11 commits into from
Dec 5, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
- Added stub test creation to `create_test_yml` ([#2476](https://github.com/nf-core/tools/pull/2476))
- Replace ModulePatch by ComponentPatch ([#2482](https://github.com/nf-core/tools/pull/2482))
- Fixed `nf-core modules lint` to work with new module structure for nf-test ([#2494](https://github.com/nf-core/tools/pull/2494))
- Add option `--migrate-pytest` to create a module with nf-test taking into account an existing module ([#2549](https://github.com/nf-core/tools/pull/2549))

### Subworkflows

- Added stub test creation to `create_test_yml` ([#2476](https://github.com/nf-core/tools/pull/2476))
- Fixed `nf-core subworkflows lint` to work with new module structure for nf-test ([#2494](https://github.com/nf-core/tools/pull/2494))
- Add option `--migrate-pytest` to create a subworkflow with nf-test taking into account an existing subworkflow ([#2549](https://github.com/nf-core/tools/pull/2549))

### General

Expand Down
21 changes: 17 additions & 4 deletions nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,20 @@ def modules_remove(ctx, dir, tool):
default=False,
help="Create a module from the template without TODOs or examples",
)
@click.option("--migrate-pytest", is_flag=True, default=False, help="Migrate a module with pytest tests to nf-test")
def create_module(
ctx, tool, dir, author, label, meta, no_meta, force, conda_name, conda_package_version, empty_template
ctx,
tool,
dir,
author,
label,
meta,
no_meta,
force,
conda_name,
conda_package_version,
empty_template,
migrate_pytest,
):
"""
Create a new DSL2 module from the nf-core template.
Expand All @@ -841,7 +853,7 @@ def create_module(
# Run function
try:
module_create = ModuleCreate(
dir, tool, author, label, has_meta, force, conda_name, conda_package_version, empty_template
dir, tool, author, label, has_meta, force, conda_name, conda_package_version, empty_template, migrate_pytest
)
module_create.create()
except UserWarning as e:
Expand Down Expand Up @@ -1033,7 +1045,8 @@ def bump_versions(ctx, tool, dir, all, show_all):
@click.option("-d", "--dir", type=click.Path(exists=True), default=".", metavar="<directory>")
@click.option("-a", "--author", type=str, metavar="<author>", help="Module author's GitHub username prefixed with '@'")
@click.option("-f", "--force", is_flag=True, default=False, help="Overwrite any files if they already exist")
def create_subworkflow(ctx, subworkflow, dir, author, force):
@click.option("--migrate-pytest", is_flag=True, default=False, help="Migrate a module with pytest tests to nf-test")
def create_subworkflow(ctx, subworkflow, dir, author, force, migrate_pytest):
"""
Create a new subworkflow from the nf-core template.

Expand All @@ -1047,7 +1060,7 @@ def create_subworkflow(ctx, subworkflow, dir, author, force):

# Run function
try:
subworkflow_create = SubworkflowCreate(dir, subworkflow, author, force)
subworkflow_create = SubworkflowCreate(dir, subworkflow, author, force, migrate_pytest)
subworkflow_create.create()
except UserWarning as e:
log.critical(e)
Expand Down
98 changes: 88 additions & 10 deletions nf_core/components/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,21 @@
import logging
import os
import re
import shutil
import subprocess
from pathlib import Path
from typing import Dict, Optional

import jinja2
import questionary
import rich
import yaml
from packaging.version import parse as parse_version

import nf_core
import nf_core.utils
from nf_core.components.components_command import ComponentCommand
from nf_core.lint_utils import run_prettier_on_file

log = logging.getLogger(__name__)

Expand All @@ -38,6 +41,7 @@ def __init__(
conda_name: Optional[str] = None,
conda_version: Optional[str] = None,
empty_template: bool = False,
migrate_pytest: bool = False,
):
super().__init__(component_type, directory)
self.directory = directory
Expand All @@ -58,6 +62,7 @@ def __init__(
self.docker_container = None
self.file_paths: Dict[str, str] = {}
self.not_empty_template = not empty_template
self.migrate_pytest = migrate_pytest

def create(self):
"""
Expand Down Expand Up @@ -136,20 +141,35 @@ def create(self):
# Check existence of directories early for fast-fail
self.file_paths = self._get_component_dirs()

if self.component_type == "modules":
# Try to find a bioconda package for 'component'
self._get_bioconda_tool()
if self.migrate_pytest:
# Rename the component directory to old
component_old_dir = self.component_dir + "_old"
component_parent_path = Path(self.directory, self.component_type, self.org)
component_old_path = component_parent_path / component_old_dir
component_path = component_parent_path / self.component_dir

# Prompt for GitHub username
self._get_username()
component_path.rename(component_old_path)
else:
if self.component_type == "modules":
# Try to find a bioconda package for 'component'
self._get_bioconda_tool()

if self.component_type == "modules":
self._get_module_structure_components()
# Prompt for GitHub username
self._get_username()

if self.component_type == "modules":
self._get_module_structure_components()

# Create component template with jinja2
self._render_template()
log.info(f"Created component template: '{self.component_name}'")

if self.migrate_pytest:
self._copy_old_files(component_old_path)
log.info("Migrate pytest tests: Copied original module files to new module")
shutil.rmtree(component_old_path)
self._print_and_delete_pytest_files()

new_files = list(self.file_paths.values())
log.info("Created following files:\n " + "\n ".join(new_files))

Expand Down Expand Up @@ -348,7 +368,7 @@ def _get_component_dirs(self):
component_dir = os.path.join(self.directory, self.component_type, self.org, self.component_dir)

# Check if module/subworkflow directories exist already
if os.path.exists(component_dir) and not self.force_overwrite:
if os.path.exists(component_dir) and not self.force_overwrite and not self.migrate_pytest:
raise UserWarning(
f"{self.component_type[:-1]} directory exists: '{component_dir}'. Use '--force' to overwrite"
)
Expand All @@ -358,7 +378,7 @@ def _get_component_dirs(self):
parent_tool_main_nf = os.path.join(
self.directory, self.component_type, self.org, self.component, "main.nf"
)
if self.subtool and os.path.exists(parent_tool_main_nf):
if self.subtool and os.path.exists(parent_tool_main_nf) and not self.migrate_pytest:
raise UserWarning(
f"Module '{parent_tool_main_nf}' exists already, cannot make subtool '{self.component_name}'"
)
Expand All @@ -367,7 +387,7 @@ def _get_component_dirs(self):
tool_glob = glob.glob(
f"{os.path.join(self.directory, self.component_type, self.org, self.component)}/*/main.nf"
)
if not self.subtool and tool_glob:
if not self.subtool and tool_glob and not self.migrate_pytest:
raise UserWarning(
f"Module subtool '{tool_glob[0]}' exists already, cannot make tool '{self.component_name}'"
)
Expand Down Expand Up @@ -411,3 +431,61 @@ def _get_username(self):
f"[violet]GitHub Username:[/]{' (@author)' if author_default is None else ''}",
default=author_default,
)

def _copy_old_files(self, component_old_path):
"""Copy files from old module to new module"""
log.debug("Copying original main.nf file")
shutil.copyfile(component_old_path / "main.nf", self.file_paths[self.component_type + "/main.nf"])
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
log.debug("Copying original meta.yml file")
shutil.copyfile(component_old_path / "meta.yml", self.file_paths[self.component_type + "/meta.yml"])
if self.component_type == "modules":
log.debug("Copying original environment.yml file")
shutil.copyfile(
component_old_path / "environment.yml", self.file_paths[self.component_type + "/environment.yml"]
)
# Create a nextflow.config file if it contains information other than publishDir
pytest_dir = Path(self.directory, "tests", self.component_type, self.org, self.component_dir)
nextflow_config = pytest_dir / "nextflow.config"
if nextflow_config.is_file():
with open(nextflow_config, "r") as fh:
config_lines = ""
for line in fh:
if "publishDir" not in line:
config_lines += line
if len(config_lines) > 0:
log.debug("Copying nextflow.config file from pytest tests")
with open(
Path(self.directory, self.component_type, self.org, self.component_dir, "tests", "nextflow.config"),
"w+",
) as ofh:
ofh.write(config_lines)

def _print_and_delete_pytest_files(self):
"""Prompt if pytest files should be deleted and printed to stdout"""
pytest_dir = Path(self.directory, "tests", self.component_type, self.org, self.component_dir)
if rich.prompt.Confirm.ask(
"[violet]Do you want to delete the pytest files?[/]\nPytest file 'main.nf' will be printed to standard output to allow migrating the tests manually to 'main.nf.test'.",
default=False,
):
with open(pytest_dir / "main.nf", "r") as fh:
log.info(fh.read())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use the Syntax function from rich to add syntax highlighting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are already using rich Logging. Not the best highlighting Nextflow but strings and functions are highlighted:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, can we make it somehow easier to copy this text? e.g. add it to the pasteboard?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need to add a new library or take into account different OS.
Also, the important parts to copy over are only the input declarations. I'm not sure if it's worth it copying the whole file to the clipboard.
The current default is to not print the content of this file and not delete it. Do you think this is easier?

shutil.rmtree(pytest_dir)
log.info(
"[yellow]Please convert the pytest tests to nf-test in 'main.nf.test'.[/]\n"
"You can find more information about nf-test [link=https://nf-co.re/docs/contributing/modules#migrating-from-pytest-to-nf-test]at the nf-core web[/link]. "
)
else:
log.info(
"[yellow]Please migrate the pytest tests to nf-test in 'main.nf.test'.[/]\n"
"You can find more information about nf-test [link=https://nf-co.re/docs/contributing/modules#migrating-from-pytest-to-nf-test]at the nf-core web[/link].\n"
f"Once done, make sure to delete the module pytest files to avoid linting errors: {pytest_dir}"
)
# Delete tags from pytest_modules.yml
modules_yml = Path(self.directory, "tests", "config", "pytest_modules.yml")
with open(modules_yml, "r") as fh:
yml_file = yaml.safe_load(fh)
yml_key = self.component_dir if self.component_type == "modules" else f"subworkflows/{self.component_dir}"
del yml_file[yml_key]
with open(modules_yml, "w") as fh:
yaml.dump(yml_file, fh)
run_prettier_on_file(modules_yml)
2 changes: 2 additions & 0 deletions nf_core/modules/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def __init__(
conda_name=None,
conda_version=None,
empty_template=False,
migrate_pytest=False,
):
super().__init__(
"modules",
Expand All @@ -29,4 +30,5 @@ def __init__(
conda_name,
conda_version,
empty_template,
migrate_pytest,
)
2 changes: 2 additions & 0 deletions nf_core/subworkflows/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ def __init__(
component="",
author=None,
force=False,
migrate_pytest=False,
):
super().__init__(
"subworkflows",
pipeline_dir,
component,
author,
force=force,
migrate_pytest=migrate_pytest,
)
76 changes: 75 additions & 1 deletion tests/modules/create.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
import filecmp
import os
import shutil
from pathlib import Path
from unittest import mock

import pytest
import requests_cache
import responses
import yaml
from git.repo import Repo

import nf_core.modules
from tests.utils import mock_anaconda_api_calls, mock_biocontainers_api_calls
from tests.utils import (
GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH,
GITLAB_URL,
mock_anaconda_api_calls,
mock_biocontainers_api_calls,
)


def test_modules_create_succeed(self):
Expand Down Expand Up @@ -65,3 +76,66 @@ def test_modules_create_nfcore_modules_subtool(self):
assert os.path.exists(
os.path.join(self.nfcore_modules, "modules", "nf-core", "star", "index", "tests", "main.nf.test")
)


@mock.patch("rich.prompt.Confirm.ask")
def test_modules_migrate(self, mock_rich_ask):
"""Create a module with the --migrate-pytest option to convert pytest to nf-test"""
pytest_dir = Path(self.nfcore_modules, "tests", "modules", "nf-core", "samtools", "sort")
module_dir = Path(self.nfcore_modules, "modules", "nf-core", "samtools", "sort")

# Clone modules repo with pytests
shutil.rmtree(self.nfcore_modules)
Repo.clone_from(GITLAB_URL, self.nfcore_modules, branch=GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH)
with open(module_dir / "main.nf", "r") as fh:
old_main_nf = fh.read()
with open(module_dir / "meta.yml", "r") as fh:
old_meta_yml = fh.read()

# Create a module with --migrate-pytest
mock_rich_ask.return_value = True
module_create = nf_core.modules.ModuleCreate(self.nfcore_modules, "samtools/sort", migrate_pytest=True)
module_create.create()

with open(module_dir / "main.nf", "r") as fh:
new_main_nf = fh.read()
with open(module_dir / "meta.yml", "r") as fh:
new_meta_yml = fh.read()
nextflow_config = module_dir / "tests" / "nextflow.config"

# Check that old files have been copied to the new module
assert old_main_nf == new_main_nf
assert old_meta_yml == new_meta_yml
assert nextflow_config.is_file()

# Check that pytest folder is deleted
assert not pytest_dir.is_dir()

# Check that pytest_modules.yml is updated
with open(Path(self.nfcore_modules, "tests", "config", "pytest_modules.yml")) as fh:
modules_yml = yaml.safe_load(fh)
assert "samtools/sort" not in modules_yml.keys()


@mock.patch("rich.prompt.Confirm.ask")
def test_modules_migrate_no_delete(self, mock_rich_ask):
"""Create a module with the --migrate-pytest option to convert pytest to nf-test.
Test that pytest directory is not deleted."""
pytest_dir = Path(self.nfcore_modules, "tests", "modules", "nf-core", "samtools", "sort")

# Clone modules repo with pytests
shutil.rmtree(self.nfcore_modules)
Repo.clone_from(GITLAB_URL, self.nfcore_modules, branch=GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH)

# Create a module with --migrate-pytest
mock_rich_ask.return_value = False
module_create = nf_core.modules.ModuleCreate(self.nfcore_modules, "samtools/sort", migrate_pytest=True)
module_create.create()

# Check that pytest folder is not deleted
assert pytest_dir.is_dir()

# Check that pytest_modules.yml is updated
with open(Path(self.nfcore_modules, "tests", "config", "pytest_modules.yml")) as fh:
modules_yml = yaml.safe_load(fh)
assert "samtools/sort" not in modules_yml.keys()
Loading
Loading