Merge pull request #3584 from mashehu/broaden-channel-linting

Author: mashehu

nf_core/modules/lint/environment_yml.py

@@ -83,7 +83,28 @@ def environment_yml(module_lint_object: ComponentLint, module: NFCoreComponent,
 
         if valid_env_yml:
             # Check that the dependencies section is sorted alphabetically
-            if sorted(env_yml["dependencies"]) == env_yml["dependencies"]:
+            def sort_recursively(obj):
+                """Simple recursive sort for nested structures."""
+                if isinstance(obj, list):
+
+                    def get_key(x):
+                        if isinstance(x, dict):
+                            # For dicts like {"pip": [...]}, use the key "pip"
+                            return (list(x.keys())[0], 1)
+                        else:
+                            # For strings like "pip=23.3.1", use "pip" and for bioconda::samtools=1.15.1, use "bioconda::samtools"
+                            return (str(x).split("=")[0], 0)
+
+                    return sorted([sort_recursively(item) for item in obj], key=get_key)
+                elif isinstance(obj, dict):
+                    return {k: sort_recursively(v) for k, v in obj.items()}
+                else:
+                    return obj
+
+            sorted_dependencies = sort_recursively(env_yml["dependencies"])
+
+            # Direct comparison of sorted vs original dependencies
+            if sorted_dependencies == env_yml["dependencies"]:
                 module.passed.append(
                     (
                         "environment_yml_sorted",
@@ -96,6 +117,6 @@ def environment_yml(module_lint_object: ComponentLint, module: NFCoreComponent,
                 log.info(
                     f"Dependencies in {module.component_name}'s environment.yml were not sorted alphabetically. Sorting them now."
                 )
-                env_yml["dependencies"].sort()
+                env_yml["dependencies"] = sorted_dependencies
                 with open(Path(module.component_dir, "environment.yml"), "w") as fh:
                     yaml.dump(env_yml, fh, Dumper=custom_yaml_dumper())

requirements.txt

@@ -7,7 +7,6 @@ jsonschema>=4.0
 markdown>=3.3
 packaging
 pillow
-pdiff
 pre-commit
 prompt_toolkit<=3.0.51
 pydantic>=2.2.1

tests/modules/test_lint.py

@@ -394,35 +394,10 @@ def test_modules_lint_snapshot_file_not_needed(self):
 
     def test_modules_environment_yml_file_doesnt_exists(self):
         """Test linting a module with an environment.yml file"""
-        Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "environment.yml").rename(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml.bak",
-            )
-        )
+        (self.bpipe_test_module_path / "environment.yml").rename(self.bpipe_test_module_path / "environment.yml.bak")
         module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
         module_lint.lint(print_results=False, module="bpipe/test")
-        Path(
-            self.nfcore_modules,
-            "modules",
-            "nf-core",
-            "bpipe",
-            "test",
-            "environment.yml.bak",
-        ).rename(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml",
-            )
-        )
+        (self.bpipe_test_module_path / "environment.yml.bak").rename(self.bpipe_test_module_path / "environment.yml")
         assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
         assert len(module_lint.passed) > 0
         assert len(module_lint.warned) >= 0
@@ -438,32 +413,13 @@ def test_modules_environment_yml_file_sorted_correctly(self):
 
     def test_modules_environment_yml_file_sorted_incorrectly(self):
         """Test linting a module with an incorrectly sorted environment.yml file"""
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml",
-            )
-        ) as fh:
+        with open(self.bpipe_test_module_path / "environment.yml") as fh:
             yaml_content = yaml.safe_load(fh)
         # Add a new dependency to the environment.yml file and reverse the order
         yaml_content["dependencies"].append("z=0.0.0")
         yaml_content["dependencies"].reverse()
         yaml_content = yaml.dump(yaml_content)
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml",
-            ),
-            "w",
-        ) as fh:
+        with open(self.bpipe_test_module_path / "environment.yml", "w") as fh:
             fh.write(yaml_content)
         module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
         module_lint.lint(print_results=False, module="bpipe/test")
@@ -474,44 +430,77 @@ def test_modules_environment_yml_file_sorted_incorrectly(self):
 
     def test_modules_environment_yml_file_not_array(self):
         """Test linting a module with an incorrectly formatted environment.yml file"""
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml",
-            )
-        ) as fh:
+        with open(self.bpipe_test_module_path / "environment.yml") as fh:
             yaml_content = yaml.safe_load(fh)
         yaml_content["dependencies"] = "z"
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "environment.yml",
-            ),
-            "w",
-        ) as fh:
+        with open(self.bpipe_test_module_path / "environment.yml", "w") as fh:
+            fh.write(yaml.dump(yaml_content))
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+        assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) > 0
+        assert len(module_lint.warned) >= 0
+        assert module_lint.failed[0].lint_test == "environment_yml_valid"
+
+    def test_modules_environment_yml_file_mixed_dependencies(self):
+        """Test linting a module with mixed-type dependencies (strings and pip dict)"""
+        with open(self.bpipe_test_module_path / "environment.yml") as fh:
+            yaml_content = yaml.safe_load(fh)
+
+        # Create mixed dependencies with strings and pip dict in wrong order
+        yaml_content["dependencies"] = [
+            "python=3.8",
+            {"pip": ["zzz-package==1.0.0", "aaa-package==2.0.0"]},
+            "bioconda::samtools=1.15.1",
+            "bioconda::fastqc=0.12.1",
+            "pip=23.3.1",
+        ]
+
+        with open(self.bpipe_test_module_path / "environment.yml", "w") as fh:
             fh.write(yaml.dump(yaml_content))
+
         module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
         module_lint.lint(print_results=False, module="bpipe/test")
+
+        # Check that the dependencies were sorted correctly
+        with open(self.bpipe_test_module_path / "environment.yml") as fh:
+            sorted_yaml = yaml.safe_load(fh)
+
+        expected_deps = [
+            "bioconda::fastqc=0.12.1",
+            "bioconda::samtools=1.15.1",
+            "pip=23.3.1",
+            {"pip": ["aaa-package==2.0.0", "zzz-package==1.0.0"]},
+            "python=3.8",
+        ]
+
+        assert sorted_yaml["dependencies"] == expected_deps
+        assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) > 0
+        assert len(module_lint.warned) >= 0
+
+    def test_modules_environment_yml_file_default_channel_fails(self):
+        """Test linting a module with a default channel set in the environment.yml file, which should fail"""
+        with open(self.bpipe_test_module_path / "environment.yml") as fh:
+            yaml_content = yaml.safe_load(fh)
+        yaml_content["channels"] = ["bioconda", "default"]
+        with open(self.bpipe_test_module_path / "environment.yml", "w") as fh:
+            fh.write(yaml.dump(yaml_content))
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
         assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
         assert len(module_lint.passed) > 0
         assert len(module_lint.warned) >= 0
         assert module_lint.failed[0].lint_test == "environment_yml_valid"
 
     def test_modules_meta_yml_incorrect_licence_field(self):
         """Test linting a module with an incorrect Licence field in meta.yml"""
-        with open(Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "meta.yml")) as fh:
+        with open(self.bpipe_test_module_path / "meta.yml") as fh:
             meta_yml = yaml.safe_load(fh)
         meta_yml["tools"][0]["bpipe"]["licence"] = "[MIT]"
         with open(
-            Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "meta.yml"),
+            self.bpipe_test_module_path / "meta.yml",
             "w",
         ) as fh:
             fh.write(yaml.dump(meta_yml))
@@ -589,45 +578,13 @@ def test_modules_missing_test_dir(self):
 
     def test_modules_missing_test_main_nf(self):
         """Test linting a module with a missing test/main.nf file"""
-        Path(
-            self.nfcore_modules,
-            "modules",
-            "nf-core",
-            "bpipe",
-            "test",
-            "tests",
-            "main.nf.test",
-        ).rename(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "tests",
-                "main.nf.test.bak",
-            )
+        (self.bpipe_test_module_path / "tests" / "main.nf.test").rename(
+            self.bpipe_test_module_path / "tests" / "main.nf.test.bak"
         )
         module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
         module_lint.lint(print_results=False, module="bpipe/test")
-        Path(
-            self.nfcore_modules,
-            "modules",
-            "nf-core",
-            "bpipe",
-            "test",
-            "tests",
-            "main.nf.test.bak",
-        ).rename(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "tests",
-                "main.nf.test",
-            )
+        (self.bpipe_test_module_path / "tests" / "main.nf.test.bak").rename(
+            self.bpipe_test_module_path / "tests" / "main.nf.test"
         )
         assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
         assert len(module_lint.passed) >= 0
@@ -666,46 +623,15 @@ def test_nftest_failing_linting(self):
 
     def test_modules_absent_version(self):
         """Test linting a nf-test module if the versions is absent in the snapshot file `"""
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "tests",
-                "main.nf.test.snap",
-            )
-        ) as fh:
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
+        with open(snap_file) as fh:
             content = fh.read()
             new_content = content.replace("versions", "foo")
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "tests",
-                "main.nf.test.snap",
-            ),
-            "w",
-        ) as fh:
+        with open(snap_file, "w") as fh:
             fh.write(new_content)
         module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
         module_lint.lint(print_results=False, module="bpipe/test")
-        with open(
-            Path(
-                self.nfcore_modules,
-                "modules",
-                "nf-core",
-                "bpipe",
-                "test",
-                "tests",
-                "main.nf.test.snap",
-            ),
-            "w",
-        ) as fh:
+        with open(snap_file, "w") as fh:
             fh.write(content)
         assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
         assert len(module_lint.passed) >= 0
@@ -714,15 +640,7 @@ def test_modules_absent_version(self):
 
     def test_modules_empty_file_in_snapshot(self):
         """Test linting a nf-test module with an empty file sha sum in the test snapshot, which should make it fail (if it is not a stub)"""
-        snap_file = Path(
-            self.nfcore_modules,
-            "modules",
-            "nf-core",
-            "bpipe",
-            "test",
-            "tests",
-            "main.nf.test.snap",
-        )
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
         snap = json.load(snap_file.open())
         content = snap_file.read_text()
         snap["my test"]["content"][0]["0"] = "test:md5,d41d8cd98f00b204e9800998ecf8427e"
@@ -743,15 +661,7 @@ def test_modules_empty_file_in_snapshot(self):
 
     def test_modules_empty_file_in_stub_snapshot(self):
         """Test linting a nf-test module with an empty file sha sum in the stub test snapshot, which should make it not fail"""
-        snap_file = Path(
-            self.nfcore_modules,
-            "modules",
-            "nf-core",
-            "bpipe",
-            "test",
-            "tests",
-            "main.nf.test.snap",
-        )
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
         snap = json.load(snap_file.open())
         content = snap_file.read_text()
         snap["my_test_stub"] = {"content": [{"0": "test:md5,d41d8cd98f00b204e9800998ecf8427e", "versions": {}}]}

tests/test_modules.py

@@ -160,6 +160,9 @@ def setUp(self):
         # Set up the nf-core/modules repo dummy
         self.nfcore_modules = create_modules_repo_dummy(self.tmp_dir)
 
+        # Common path to the bpipe/test module used in tests
+        self.bpipe_test_module_path = Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test")
+
     def test_modulesrepo_class(self):
         """Initialise a modules repo object"""
         modrepo = nf_core.modules.modules_repo.ModulesRepo()