Make the lint and type checks actually run

.github/workflows/python-packages.yaml

@@ -31,6 +31,28 @@ jobs:
       run: |
         cd packages/python/${{ matrix.package }}
         tox -e lint
+
+  type:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        package: [openproblems]
+
+    steps:
+    - uses: actions/checkout@v7
+
+    - name: Using Python 3.10
+      uses: actions/setup-python@v7
+      with:
+        python-version: "3.10"
+
+    - name: Install tox
+      run: python -m pip install tox
+
+    - name: Run tox type
+      run: |
+        cd packages/python/${{ matrix.package }}
+        tox -e type
   test:
     runs-on: ubuntu-latest
 

packages/python/openproblems/CHANGELOG.md

@@ -24,6 +24,12 @@
 
 * `check_config`: Skip the Nextflow resource label check for components whose script is itself a Nextflow workflow. Viash renders those as a workflow rather than a process, so the labels would have no effect.
 
+* `tox -e lint` now checks the formatting rather than applying it, so the lint job can actually fail.
+  Run `black` yourself, or `tox -e lint -- .`, to apply the changes.
+
+* The `type` environment is no longer skipped in CI, and the 25 type errors it had accumulated
+  are fixed.
+
 ## BUG FIXES
 
 * `read_task_metadata`: Order the task graph topologically instead of by a breadth-first search from a single root.

packages/python/openproblems/pyproject.toml

@@ -51,3 +51,17 @@ exclude = ["tests*"]
 
 [tool.setuptools_scm]
 root = "../../.."
+
+# the component tests import these inside the function that needs them, so that
+# a task repo only has to provide the ones its own components use
+[[tool.mypy.overrides]]
+module = [
+  "anndata.*",
+  "networkx.*",
+  "pandas.*",
+  "requests.*",
+  "spatialdata.*",
+  "urllib3.*",
+  "yaml.*",
+]
+ignore_missing_imports = true

packages/python/openproblems/src/openproblems/project/component_tests/check_config.py

@@ -60,9 +60,7 @@ def check_references(references: Dict[str, Union[str, List[str]]]) -> None:
             assert re.match(r"^@.*{.*", b), f"Invalid bibtex format: {b}"
 
 
-def check_links(
-    links: Dict[str, Union[str, List[str]]], required: List[str] = []
-) -> None:
+def check_links(links: Dict[str, str], required: List[str] = []) -> None:
     links = links or {}
 
     for expected_link in required:
@@ -122,13 +120,18 @@ def check_config(config: dict) -> None:
     print("Check that .namespace is defined", flush=True)
     assert config.get("namespace"), ".namespace is not defined"
 
-    print("Check that .info.type is 'method', 'control_method', or 'metric'", flush=True)
+    print(
+        "Check that .info.type is 'method', 'control_method', or 'metric'", flush=True
+    )
     expected_types = ["method", "control_method", "metric"]
     assert (
         comp_type in expected_types
     ), f".info.type is '{comp_type}' but should be one of: {', '.join(expected_types)}"
 
-    print("Check component metadata fields (name, label, summary, description)", flush=True)
+    print(
+        "Check component metadata fields (name, label, summary, description)",
+        flush=True,
+    )
     if comp_type == "metric":
         metric_infos = info.get("metrics", [])
         assert metric_infos, ".info.metrics is not defined"
@@ -138,7 +141,10 @@ def check_config(config: dict) -> None:
         check_info(info, config, comp_type=comp_type)
 
     if "preferred_normalization" in info:
-        print("Check that .info.preferred_normalization is a valid normalization method", flush=True)
+        print(
+            "Check that .info.preferred_normalization is a valid normalization method",
+            flush=True,
+        )
         norm_methods = [
             "log_cpm",
             "log_cp10k",
@@ -155,7 +161,9 @@ def check_config(config: dict) -> None:
         )
 
     if "variants" in info:
-        print("Check that .info.variants only references valid argument names", flush=True)
+        print(
+            "Check that .info.variants only references valid argument names", flush=True
+        )
         arg_names = [arg["clean_name"] for arg in config["all_arguments"]] + [
             "preferred_normalization"
         ]
@@ -185,7 +193,9 @@ def check_config(config: dict) -> None:
     )
 
     if not is_nextflow_workflow:
-        print("Check that the Nextflow runner has time, mem, and cpu labels", flush=True)
+        print(
+            "Check that the Nextflow runner has time, mem, and cpu labels", flush=True
+        )
         assert nextflow_runner.get(
             "directives"
         ), "directives not a field in nextflow runner"

packages/python/openproblems/src/openproblems/project/component_tests/run_and_check_output.py

@@ -22,8 +22,12 @@ def check_input_files(arguments: list) -> None:
     for arg in arguments:
         if arg["type"] == "file" and arg["direction"] == "input" and arg["required"]:
             expected_path = arg.get("value")
-            assert expected_path is not None, f"Input argument '{arg['name']}' is missing a value"
-            assert not arg["must_exist"] or path.exists(expected_path), f"Input file '{expected_path}' does not exist"
+            assert (
+                expected_path is not None
+            ), f"Input argument '{arg['name']}' is missing a value"
+            assert not arg["must_exist"] or path.exists(
+                expected_path
+            ), f"Input file '{expected_path}' does not exist"
 
 
 def check_output_files(arguments: list) -> None:
@@ -34,8 +38,12 @@ def check_output_files(arguments: list) -> None:
     for arg in arguments:
         if arg["type"] == "file" and arg["direction"] == "output" and arg["required"]:
             expected_path = arg.get("value")
-            assert expected_path is not None, f"Output argument '{arg['name']}' is missing a value"
-            assert not arg["must_exist"] or path.exists(expected_path), f"Output file '{expected_path}' does not exist"
+            assert (
+                expected_path is not None
+            ), f"Output argument '{arg['name']}' is missing a value"
+            assert not arg["must_exist"] or path.exists(
+                expected_path
+            ), f"Output file '{expected_path}' does not exist"
 
     print(">> Validating the contents and format of output files", flush=True)
     for arg in arguments:

packages/python/openproblems/src/openproblems/project/read_nested_yaml.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from typing import Any
+
 
 def read_nested_yaml(path: str, project_path: str | None = None) -> dict:
     """
@@ -35,8 +37,8 @@ def read_nested_yaml(path: str, project_path: str | None = None) -> dict:
 
 
 def process_nested_yaml(
-    data: any, root_data: dict, path: str, project_path: str
-) -> dict:
+    data: Any, root_data: dict, path: str, project_path: str | None
+) -> Any:
     """
     Process the merge keys in a YAML
 
@@ -62,7 +64,7 @@ def process_nested_yaml(
             for k, v in data.items()
         }
 
-        new_data = {}
+        new_data: Any = {}
         if "__merge__" in processed_data and not isinstance(
             processed_data["__merge__"], dict
         ):

packages/python/openproblems/src/openproblems/utils/deep_merge.py

@@ -1,4 +1,7 @@
-def deep_merge(obj1: any, obj2: any) -> dict:
+from typing import Any
+
+
+def deep_merge(obj1: Any, obj2: Any) -> Any:
     """Recursively merge two dictionaries or lists.
 
     Args:

packages/python/openproblems/tox.ini

@@ -17,11 +17,12 @@ description = run linters
 skip_install = true
 deps =
     black==22.12
-commands = black {posargs:.}
+commands = black {posargs:--check --diff .}
 
 [testenv:type]
 description = run type checks
 deps =
     mypy>=0.991
+    pytest>=7
 commands =
     mypy {posargs:src tests}