Skip to content

Commit

Permalink
Merge pull request #3256 from awgymer/1929-local-modules-use-remote-f…
Browse files Browse the repository at this point in the history
…ormat

1929: local modules use remote format from template
  • Loading branch information
awgymer authored Jan 24, 2025
2 parents 583809c + 262dea3 commit bc3417a
Show file tree
Hide file tree
Showing 21 changed files with 338 additions and 96 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@

### Modules

- 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

- Fix `process.shell` in `nextflow.config` ([#3416](https://github.com/nf-core/tools/pull/3416))
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
30 changes: 19 additions & 11 deletions nf_core/components/nfcore_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,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 @@ -85,15 +85,23 @@ 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.component_dir = self.component_dir.parent
self.meta_yml = None
self.environment_yml = None
self.nftest_testdir = None
self.nftest_main_nf = None

self.process_name: str = self._get_process_name()

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 @@ -57,8 +63,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
5 changes: 4 additions & 1 deletion nf_core/pipelines/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from nf_core import __version__
from nf_core.components.lint import ComponentLint
from nf_core.pipelines.lint_utils import console
from nf_core.utils import NFCoreYamlConfig, NFCoreYamlLintConfig, strip_ansi_codes
from nf_core.utils import NFCoreYamlLintConfig, strip_ansi_codes
from nf_core.utils import plural_s as _s

from .actions_awsfulltest import actions_awsfulltest
Expand All @@ -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

0 comments on commit bc3417a

Please sign in to comment.