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

1929: local modules use remote format from template #3256

Draft
wants to merge 9 commits into
base: dev
Choose a base branch
from
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
### Modules

- add a panel around diff previews when updating ([#3246](https://github.com/nf-core/tools/pull/3246))
- Modules created in pipelines "local" dir now use the full template ([#3256](https://github.com/nf-core/tools/pull/3256))

### Subworkflows

- Subworkflows created in pipelines "local" dir now use the full template ([#3256](https://github.com/nf-core/tools/pull/3256))

### General

- Include .nf-core.yml in `nf-core pipelines bump-version` ([#3220](https://github.com/nf-core/tools/pull/3220))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# modules_structure

```{eval-rst}
.. automethod:: nf_core.pipelines.lint.PipelineLint.local_component_structure
```
4 changes: 4 additions & 0 deletions nf_core/components/components_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ def get_local_components(self) -> List[str]:
"""
local_component_dir = Path(self.directory, self.component_type, "local")
return [
str(Path(directory).relative_to(local_component_dir))
for directory, _, files in os.walk(local_component_dir)
if "main.nf" in files
] + [
str(path.relative_to(local_component_dir)) for path in local_component_dir.iterdir() if path.suffix == ".nf"
]

Expand Down
96 changes: 36 additions & 60 deletions nf_core/components/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ def create(self) -> bool:
e.g bam_sort or bam_sort_samtools, respectively.

If <directory> is a pipeline, this function creates a file called:
'<directory>/modules/local/tool.nf'
'<directory>/modules/local/tool/main.nf'
OR
'<directory>/modules/local/tool_subtool.nf'
'<directory>/modules/local/tool/subtool/main.nf'
OR for subworkflows
'<directory>/subworkflows/local/subworkflow_name.nf'
'<directory>/subworkflows/local/subworkflow_name/main.nf'

If <directory> is a clone of nf-core/modules, it creates or modifies the following files:

Expand Down Expand Up @@ -355,70 +355,46 @@ def _get_component_dirs(self) -> Dict[str, Path]:
"""
file_paths = {}
if self.repo_type == "pipeline":
local_component_dir = Path(self.directory, self.component_type, "local")
# Check whether component file already exists
component_file = local_component_dir / f"{self.component_name}.nf"
if component_file.exists() and not self.force_overwrite:
raise UserWarning(
f"{self.component_type[:-1].title()} file exists already: '{component_file}'. Use '--force' to overwrite"
)

if self.component_type == "modules":
# If a subtool, check if there is a module called the base tool name already
if self.subtool and (local_component_dir / f"{self.component}.nf").exists():
raise UserWarning(
f"Module '{self.component}' exists already, cannot make subtool '{self.component_name}'"
)

# If no subtool, check that there isn't already a tool/subtool
tool_glob = glob.glob(f"{local_component_dir}/{self.component}_*.nf")
if not self.subtool and tool_glob:
raise UserWarning(
f"Module subtool '{tool_glob[0]}' exists already, cannot make tool '{self.component_name}'"
)

# Set file paths
file_paths["main.nf"] = component_file
component_dir = Path(self.directory, self.component_type, "local", self.component_dir)

elif self.repo_type == "modules":
component_dir = Path(self.directory, self.component_type, self.org, self.component_dir)
# Check if module/subworkflow directories exist already
if component_dir.exists() 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"
)
else:
raise ValueError("`repo_type` not set correctly")

if self.component_type == "modules":
# If a subtool, check if there is a module called the base tool name already
parent_tool_main_nf = Path(
self.directory,
self.component_type,
self.org,
self.component,
"main.nf",
# Check if module/subworkflow directories exist already
if component_dir.exists() 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"
)

if self.component_type == "modules":
# If a subtool, check if there is a module called the base tool name already
parent_tool_main_nf = Path(
self.directory,
self.component_type,
self.org,
self.component,
"main.nf",
)
if self.subtool and parent_tool_main_nf.exists() and not self.migrate_pytest:
raise UserWarning(
f"Module '{parent_tool_main_nf}' exists already, cannot make subtool '{self.component_name}'"
)
if self.subtool and parent_tool_main_nf.exists() and not self.migrate_pytest:
raise UserWarning(
f"Module '{parent_tool_main_nf}' exists already, cannot make subtool '{self.component_name}'"
)

# If no subtool, check that there isn't already a tool/subtool
tool_glob = glob.glob(
f"{Path(self.directory, self.component_type, self.org, self.component)}/*/main.nf"
# If no subtool, check that there isn't already a tool/subtool
tool_glob = glob.glob(f"{Path(self.directory, self.component_type, self.org, self.component)}/*/main.nf")
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}'"
)
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}'"
)
# Set file paths
# For modules - can be tool/ or tool/subtool/ so can't do in template directory structure
file_paths["main.nf"] = component_dir / "main.nf"
file_paths["meta.yml"] = component_dir / "meta.yml"
if self.component_type == "modules":
file_paths["environment.yml"] = component_dir / "environment.yml"
file_paths["tests/main.nf.test.j2"] = component_dir / "tests" / "main.nf.test"
else:
raise ValueError("`repo_type` not set correctly")
# Set file paths
# For modules - can be tool/ or tool/subtool/ so can't do in template directory structure
file_paths["main.nf"] = component_dir / "main.nf"
file_paths["meta.yml"] = component_dir / "meta.yml"
if self.component_type == "modules":
file_paths["environment.yml"] = component_dir / "environment.yml"
file_paths["tests/main.nf.test.j2"] = component_dir / "tests" / "main.nf.test"

return file_paths

Expand Down
4 changes: 4 additions & 0 deletions nf_core/components/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ def _set_registry(self, registry) -> None:
self.registry = registry
log.debug(f"Registry set to {self.registry}")

@property
def local_module_exclude_tests(self):
return ["module_version", "module_changes", "modules_patch"]

@staticmethod
def get_all_module_lint_tests(is_pipeline):
if is_pipeline:
Expand Down
29 changes: 18 additions & 11 deletions nf_core/components/nfcore_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ def __init__(
repo_dir = self.component_dir.parts[:name_index][-1]

self.org = repo_dir
self.nftest_testdir = Path(self.component_dir, "tests")
self.nftest_main_nf = Path(self.nftest_testdir, "main.nf.test")
self.nftest_testdir: Optional[Path] = Path(self.component_dir, "tests")
self.nftest_main_nf: Optional[Path] = Path(self.nftest_testdir, "main.nf.test")

if self.repo_type == "pipeline":
patch_fn = f"{self.component_name.replace('/', '-')}.diff"
Expand All @@ -86,15 +86,22 @@ def __init__(
self.patch_path = patch_path
else:
# The main file is just the local module
self.main_nf = self.component_dir
self.component_name = self.component_dir.stem
# These attributes are only used by nf-core modules
# so just initialize them to None
self.meta_yml = None
self.environment_yml = None
self.test_dir = None
self.test_yml = None
self.test_main_nf = None
if self.component_dir.is_dir():
self.main_nf = Path(self.component_dir, "main.nf")
self.component_name = self.component_dir.stem
# These attributes are only required by nf-core modules
# so just set them to None if they don't exist
self.meta_yml = p if (p := Path(self.component_dir, "meta.yml")).exists() else None
self.environment_yml = p if (p := Path(self.component_dir, "environment.yml")).exists() else None
self.nftest_testdir = p if (p := Path(self.component_dir, "tests")).exists() else None
if self.nftest_testdir is not None:
self.nftest_main_nf = p if (p := Path(self.nftest_testdir, "main.nf.test")).exists() else None
else:
self.main_nf = self.component_dir
self.meta_yml = None
self.environment_yml = None
self.nftest_testdir = None
self.nftest_main_nf = None

def __repr__(self) -> str:
return f"<NFCoreComponent {self.component_name} {self.component_dir} {self.repo_url}>"
Expand Down
22 changes: 19 additions & 3 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def lint(
"""
# TODO: consider unifying modules and subworkflows lint() function and add it to the ComponentLint class
# Prompt for module or all
if module is None and not all_modules and len(self.all_remote_components) > 0:
if module is None and not (local or all_modules) and len(self.all_remote_components) > 0:
questions = [
{
"type": "list",
Expand Down Expand Up @@ -170,7 +170,7 @@ def lint(
self.lint_modules(local_modules, registry=registry, local=True, fix_version=fix_version)

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

if print_results:
Expand Down Expand Up @@ -234,7 +234,23 @@ def lint_module(
# TODO: consider unifying modules and subworkflows lint_module() function and add it to the ComponentLint class
# Only check the main script in case of a local module
if local:
self.main_nf(mod, fix_version, self.registry, progress_bar)
mod.get_inputs_from_main_nf()
mod.get_outputs_from_main_nf()
# Update meta.yml file if requested
if self.fix and mod.meta_yml is not None:
self.update_meta_yml_file(mod)

for test_name in self.lint_tests:
if test_name in self.local_module_exclude_tests:
continue
if test_name == "main_nf":
getattr(self, test_name)(mod, fix_version, self.registry, progress_bar)
elif test_name in ["meta_yml", "environment_yml"]:
# Allow files to be missing for local
getattr(self, test_name)(mod, allow_missing=True)
else:
getattr(self, test_name)(mod)

self.passed += [LintResult(mod, *m) for m in mod.passed]
warned = [LintResult(mod, *m) for m in (mod.warned + mod.failed)]
if not self.fail_warned:
Expand Down
11 changes: 10 additions & 1 deletion nf_core/modules/lint/environment_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
log = logging.getLogger(__name__)


def environment_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None:
def environment_yml(module_lint_object: ComponentLint, module: NFCoreComponent, allow_missing: bool = False) -> None:
"""
Lint an ``environment.yml`` file.

Expand All @@ -23,6 +23,15 @@ def environment_yml(module_lint_object: ComponentLint, module: NFCoreComponent)
env_yml = None
# load the environment.yml file
if module.environment_yml is None:
if allow_missing:
module.warned.append(
(
"environment_yml_exists",
"Module's `environment.yml` does not exist",
Path(module.component_dir, "environment.yml"),
),
)
return
raise LintExceptionError("Module does not have an `environment.yml` file")
try:
with open(module.environment_yml) as fh:
Expand Down
12 changes: 8 additions & 4 deletions nf_core/modules/lint/meta_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
log = logging.getLogger(__name__)


def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None:
def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent, allow_missing: bool = False) -> None:
"""
Lint a ``meta.yml`` file

Expand Down Expand Up @@ -42,7 +42,13 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
module (NFCoreComponent): The module to lint

"""

if module.meta_yml is None:
if allow_missing:
module.warned.append(
("meta_yml_exists", "Module `meta.yml` does not exist", Path(module.component_dir, "meta.yml"))
)
return
raise LintExceptionError("Module does not have a `meta.yml` file")
# Check if we have a patch file, get original file in that case
meta_yaml = read_meta_yml(module_lint_object, module)
if module.is_patched and module_lint_object.modules_repo.repo_path is not None:
Expand All @@ -56,8 +62,6 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
if lines is not None:
yaml = ruamel.yaml.YAML()
meta_yaml = yaml.safe_load("".join(lines))
if module.meta_yml is None:
raise LintExceptionError("Module does not have a `meta.yml` file")
if meta_yaml is None:
module.failed.append(("meta_yml_exists", "Module `meta.yml` does not exist", module.meta_yml))
return
Expand Down
27 changes: 26 additions & 1 deletion nf_core/modules/lint/module_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,44 @@

import yaml

from nf_core.components.lint import LintExceptionError
from nf_core.components.nfcore_component import NFCoreComponent

log = logging.getLogger(__name__)


def module_tests(_, module: NFCoreComponent):
def module_tests(_, module: NFCoreComponent, allow_missing: bool = False):
"""
Lint the tests of a module in ``nf-core/modules``

It verifies that the test directory exists
and contains a ``main.nf.test`` and a ``main.nf.test.snap``

"""
if module.nftest_testdir is None:
if allow_missing:
module.warned.append(
(
"test_dir_exists",
"nf-test directory is missing",
Path(module.component_dir, "tests"),
)
)
return
raise LintExceptionError("Module does not have a `tests` dir")

if module.nftest_main_nf is None:
if allow_missing:
module.warned.append(
(
"test_main_nf_exists",
"test `main.nf.test` does not exist",
Path(module.component_dir, "tests", "main.nf.test"),
)
)
return
raise LintExceptionError("Module does not have a `tests` dir")

repo_dir = module.component_dir.parts[: module.component_dir.parts.index(module.component_name.split("/")[0])][-1]
test_dir = Path(module.base_dir, "tests", "modules", repo_dir, module.component_name)
pytest_main_nf = Path(test_dir, "main.nf")
Expand Down
3 changes: 3 additions & 0 deletions nf_core/pipelines/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from .files_exist import files_exist
from .files_unchanged import files_unchanged
from .included_configs import included_configs
from .local_component_structure import local_component_structure
from .merge_markers import merge_markers
from .modules_json import modules_json
from .modules_structure import modules_structure
Expand Down Expand Up @@ -89,6 +90,7 @@ class PipelineLint(nf_core.utils.Pipeline):
merge_markers = merge_markers
modules_json = modules_json
modules_structure = modules_structure
local_component_structure = local_component_structure
multiqc_config = multiqc_config
nextflow_config = nextflow_config
nfcore_yml = nfcore_yml
Expand Down Expand Up @@ -151,6 +153,7 @@ def _get_all_lint_tests(release_mode):
"modules_json",
"multiqc_config",
"modules_structure",
"local_component_structure",
"base_config",
"modules_config",
"nfcore_yml",
Expand Down
Loading
Loading