1929: local modules use remote format from template

CHANGELOG.md

@@ -33,6 +33,7 @@
 - Update GitHub Actions ([#3237](https://github.com/nf-core/tools/pull/3237))
 - add `--dir/-d` option to schema commands ([#3247](https://github.com/nf-core/tools/pull/3247))
 - Update pre-commit hook astral-sh/ruff-pre-commit to v0.7.1 ([#3250](https://github.com/nf-core/tools/pull/3250))
+- fix(1929): more type-gating ([#3256](https://github.com/nf-core/tools/pull/3256))
 
 ## [v3.0.2 - Titanium Tapir Patch](https://github.com/nf-core/tools/releases/tag/3.0.2) - [2024-10-11]
 

docs/api/_src/pipeline_lint_tests/local_component_structure.md

@@ -0,0 +1,5 @@
+# modules_structure
+
+```{eval-rst}
+.. automethod:: nf_core.pipelines.lint.PipelineLint.local_component_structure
+```

nf_core/components/components_command.py

@@ -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"
         ]
 

nf_core/components/create.py

@@ -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:
 
@@ -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
 

nf_core/components/nfcore_component.py

@@ -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"
@@ -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}>"

nf_core/modules/lint/module_tests.py

@@ -22,6 +22,8 @@ def module_tests(_, module: NFCoreComponent):
     and contains a ``main.nf.test`` and a ``main.nf.test.snap``
 
     """
+    if module.nftest_testdir is None or module.nftest_main_nf is None:
+        raise ValueError()
     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")

nf_core/pipelines/lint/__init__.py

@@ -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
@@ -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
@@ -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",

nf_core/pipelines/lint/local_component_structure.py

@@ -0,0 +1,31 @@
+import logging
+from pathlib import Path
+
+log = logging.getLogger(__name__)
+
+
+def local_component_structure(self):
+    """
+    Check that the local modules and subworkflows directories in a pipeline have the correct format:
+
+    .. code-block:: bash
+
+        modules/local/TOOL/SUBTOOL
+
+    Prior to nf-core/tools release 3.0.3 the directory structure allowed top-level `*.nf` files:
+
+    .. code-block:: bash
+
+        modules/local/modules/TOOL_SUBTOOL.nf
+    """
+    warned = []
+    for nf_file in Path(self.wf_path, "modules", "local").glob("*.nf"):
+        warned.append(f"{nf_file.name} in modules/local should be moved to a TOOL/SUBTOOL/main.nf structure")
+    for nf_file in Path(self.wf_path, "subworkflows", "local").glob("*.nf"):
+        warned.append(f"{nf_file.name} in subworkflows/local should be moved to a TOOL/SUBTOOL/main.nf structure")
+
+    # If there are modules installed in the wrong location
+    passed = []
+    if len(warned) == 0:
+        passed = ["modules directory structure is correct 'modules/nf-core/TOOL/SUBTOOL'"]
+    return {"passed": passed, "warned": warned, "failed": [], "ignored": []}

nf_core/subworkflows/lint/subworkflow_tests.py

@@ -23,7 +23,8 @@ def subworkflow_tests(_, subworkflow: NFCoreComponent):
 
     Additionally, checks that all included components in test ``main.nf`` are specified in ``test.yml``
     """
-
+    if subworkflow.nftest_testdir is None or subworkflow.nftest_main_nf is None:
+        raise ValueError()
     repo_dir = subworkflow.component_dir.parts[
         : subworkflow.component_dir.parts.index(subworkflow.component_name.split("/")[0])
     ][-1]

tests/modules/test_create.py

@@ -31,7 +31,7 @@ def test_modules_create_succeed(self):
             )
             with requests_cache.disabled():
                 module_create.create()
-        assert os.path.exists(os.path.join(self.pipeline_dir, "modules", "local", "trimgalore.nf"))
+        assert os.path.exists(os.path.join(self.pipeline_dir, "modules", "local", "trimgalore/main.nf"))
 
     def test_modules_create_fail_exists(self):
         """Fail at creating the same module twice"""
@@ -46,7 +46,7 @@ def test_modules_create_fail_exists(self):
             with pytest.raises(UserWarning) as excinfo:
                 with requests_cache.disabled():
                     module_create.create()
-        assert "Module file exists already" in str(excinfo.value)
+        assert "module directory exists:" in str(excinfo.value)
 
     def test_modules_create_nfcore_modules(self):
         """Create a module in nf-core/modules clone"""

tests/subworkflows/test_create.py

@@ -19,7 +19,7 @@ def test_subworkflows_create_succeed(self):
             self.pipeline_dir, "test_subworkflow_local", "@author", True
         )
         subworkflow_create.create()
-        assert Path(self.pipeline_dir, "subworkflows", "local", "test_subworkflow_local.nf").exists()
+        assert Path(self.pipeline_dir, "subworkflows", "local", "test_subworkflow_local/main.nf").exists()
 
     def test_subworkflows_create_fail_exists(self):
         """Fail at creating the same subworkflow twice"""
@@ -29,7 +29,7 @@ def test_subworkflows_create_fail_exists(self):
         subworkflow_create.create()
         with pytest.raises(UserWarning) as excinfo:
             subworkflow_create.create()
-        assert "Subworkflow file exists already" in str(excinfo.value)
+        assert "subworkflow directory exists" in str(excinfo.value)
 
     def test_subworkflows_create_nfcore_modules(self):
         """Create a subworkflow in nf-core/modules clone"""