Update all annotations and check with Mypy (#520)

* add to pre-commit

* update

* Update CHANGELOG.md

* update

* update more

* skip annotations in tests

* update last ones

* update last ones

.pre-commit-config.yaml

@@ -22,3 +22,9 @@ repos:
   - id: codespell
     additional_dependencies:
     - tomli
+
+-   repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.11.2
+    hooks:
+    -   id: mypy
+        additional_dependencies: ["types-PyYAML", "types-setuptools"]

CHANGELOG.md

@@ -14,7 +14,7 @@
 * Simplified the `nwbinspector.testing` configuration framework. [#509](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/509)
 * Cleaned old references to non-recent PyNWB and HDMF versions. Current policy is that latest NWB Inspector releases should only support compatibility with latest PyNWB and HDMF. [#510](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/510)
 * Swapped setup approach to the modern `pyproject.toml` standard. [#507](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/507)
-
+* Added complete annotation typing and integrated Mypy into pre-commit. [#520](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/520)
 
 
 

docs/conf.py

@@ -8,7 +8,7 @@
 sys.path.append(str(Path(__file__).parent))
 from conf_extlinks import extlinks, intersphinx_mapping
 
-sys.path.insert(0, Path(__file__).resolve().parents[1])
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
 
 project = "NWBInspector"
 copyright = "2022-2024, CatalystNeuro"

pyproject.toml

@@ -92,7 +92,17 @@ extend-exclude = '''
 exclude = ["docs/*"]
 
 [tool.ruff.lint]
-select = ["F401", "F541", "I"] # TODO: eventually, expand to other 'D', and other 'F' linting
+# TODO: eventually, expand to other 'D', and other 'F' linting
+select = [
+    "F401",
+    "F541",
+    "I",
+    "ANN001",
+    "ANN201",
+    "ANN202",
+    "ANN205",
+    "ANN206",
+]
 fixable = ["ALL"]
 
 [tool.ruff.lint.per-file-ignores]
@@ -105,6 +115,7 @@ fixable = ["ALL"]
 "src/nwbinspector/inspector_tools/__init__.py" = ["F401", "I"]
 "src/nwbinspector/version/__init__.py" = ["F401", "I"]
 "src/nwbinspector/register_checks/__init__.py" = ["F401", "I"]
+"tests/*" = ["ANN"]
 
 [tool.ruff.lint.isort]
 relative-imports-order = "closest-to-furthest"

src/nwbinspector/_configuration.py

@@ -3,41 +3,40 @@
 import json
 from pathlib import Path
 from types import FunctionType
-from typing import Optional
+from typing import Optional, Union
 
 import jsonschema
 import yaml
+from typing_extensions import Callable
 
-from nwbinspector.utils import PathType
+from ._registration import Importance, available_checks
 
-from . import available_checks
-from ._registration import Importance
-
-INTERNAL_CONFIGS = dict(
+INTERNAL_CONFIGS: dict[str, Path] = dict(
     dandi=Path(__file__).parent / "_internal_configs" / "dandi.inspector_config.yaml",
 )
 
 
-def validate_config(config: dict):
+def validate_config(config: dict) -> None:
     """Validate an instance of configuration against the official schema."""
     config_schema_file_path = Path(__file__).parent / "_internal_configs" / "config.schema.json"
     with open(file=config_schema_file_path, mode="r") as fp:
         schema = json.load(fp=fp)
     jsonschema.validate(instance=config, schema=schema)
 
 
-def _copy_function(function):
+def _copy_function(function: Callable) -> Callable:
     """Copy the core parts of a given function, excluding wrappers, then return a new function."""
     copied_function = FunctionType(
         function.__code__, function.__globals__, function.__name__, function.__defaults__, function.__closure__
     )
 
     # in case f was given attrs (note this dict is a shallow copy)
     copied_function.__dict__.update(function.__dict__)
+
     return copied_function
 
 
-def copy_check(check):
+def copy_check(check: Callable) -> Callable:
     """
     Copy a check function so that internal attributes can be adjusted without changing the original function.
 
@@ -47,26 +46,28 @@ def copy_check(check):
     see https://github.com/NeurodataWithoutBorders/nwbinspector/pull/218 for explanation.
     """
     copied_check = _copy_function(function=check)
-    copied_check.__wrapped__ = _copy_function(function=check.__wrapped__)
+    copied_check.__wrapped__ = _copy_function(function=check.__wrapped__)  # type: ignore
+
     return copied_check
 
 
-def load_config(filepath_or_keyword: PathType) -> dict:
+def load_config(filepath_or_keyword: Union[str, Path]) -> dict:
     """
     Load a config dictionary either via keyword search of the internal configs, or an explicit filepath.
 
     Currently supported keywords are:
         - 'dandi'
             For all DANDI archive related practices, including validation and upload.
     """
-    file = INTERNAL_CONFIGS.get(filepath_or_keyword, filepath_or_keyword)
+    file = INTERNAL_CONFIGS.get(str(filepath_or_keyword), filepath_or_keyword)
     with open(file=file, mode="r") as stream:
         config = yaml.safe_load(stream=stream)
+
     return config
 
 
 def configure_checks(
-    checks: list = available_checks,
+    checks: Optional[list] = None,
     config: Optional[dict] = None,
     ignore: Optional[list[str]] = None,
     select: Optional[list[str]] = None,
@@ -77,12 +78,12 @@ def configure_checks(
 
     Parameters
     ----------
+    checks : list of check functions, optional
+        If None, defaults to all registered checks.
     config : dict
         Dictionary valid against our JSON configuration schema.
         Can specify a mapping of importance levels and list of check functions whose importance you wish to change.
         Typically loaded via json.load from a valid .json file
-    checks : list of check functions
-        Defaults to all registered checks.
     ignore: list, optional
         Names of functions to skip.
     select: list, optional
@@ -100,16 +101,19 @@ def configure_checks(
 
         The default is the lowest level, BEST_PRACTICE_SUGGESTION.
     """
+    checks = checks or available_checks
+
     if ignore is not None and select is not None:
         raise ValueError("Options 'ignore' and 'select' cannot both be used.")
     if importance_threshold not in Importance:
         raise ValueError(
             f"Indicated importance_threshold ({importance_threshold}) is not a valid importance level! Please choose "
             "from [CRITICAL_IMPORTANCE, BEST_PRACTICE_VIOLATION, BEST_PRACTICE_SUGGESTION]."
         )
+
+    checks_out: list = []
     if config is not None:
         validate_config(config=config)
-        checks_out = []
         ignore = ignore or []
         for check in checks:
             mapped_check = copy_check(check=check)
@@ -118,7 +122,7 @@ def configure_checks(
                     if importance_name == "SKIP":
                         ignore.append(check.__name__)
                         continue
-                    mapped_check.importance = Importance[importance_name]
+                    mapped_check.importance = Importance[importance_name]  # type: ignore
                     # Output wrappers are apparently parsed at time of wrapping not of time of output return...
                     # Attempting to re-wrap the copied function if the importance level is being adjusted...
                     # From https://github.com/NeurodataWithoutBorders/nwbinspector/issues/302
@@ -133,5 +137,6 @@ def configure_checks(
     elif ignore:
         checks_out = [x for x in checks_out if x.__name__ not in ignore]
     if importance_threshold:
-        checks_out = [x for x in checks_out if x.importance.value >= importance_threshold.value]
+        checks_out = [x for x in checks_out if x.importance.value >= importance_threshold.value]  # type: ignore
+
     return checks_out

src/nwbinspector/_dandi_inspection.py

@@ -21,7 +21,7 @@ def inspect_dandiset(
     importance_threshold: Union[str, Importance] = Importance.BEST_PRACTICE_SUGGESTION,
     skip_validate: bool = False,
     show_progress_bar: bool = True,
-    client: Union["dandi.dandiapi.DandiAPIClient", None] = None,
+    client: Union["dandi.dandiapi.DandiAPIClient", None] = None,  # type: ignore
 ) -> Iterable[Union[InspectorMessage, None]]:
     """
     Inspect a Dandiset for common issues.
@@ -73,12 +73,6 @@ def inspect_dandiset(
 
     dandiset = client.get_dandiset(dandiset_id=dandiset_id, version_id=dandiset_version)
 
-    # if not any(
-    #     asset_type.get("identifier", "") != "RRID:SCR_015242"  # Identifier for NWB standard
-    #     for asset_type in dandiset.get_raw_metadata().get("assetsSummary", {}).get("dataStandard", [])
-    # ):
-    #     return None
-
     nwb_assets = [asset for asset in dandiset.get_assets() if ".nwb" in pathlib.Path(asset.path).suffixes]
 
     nwb_assets_iterator = nwb_assets
@@ -101,7 +95,7 @@ def inspect_dandiset(
             importance_threshold=importance_threshold,
             skip_validate=skip_validate,
         ):
-            message.file_path = asset.path
+            message.file_path = asset.path  # type: ignore
             yield message
 
 
@@ -110,13 +104,13 @@ def inspect_dandi_file_path(
     dandi_file_path: str,
     dandiset_id: str,
     dandiset_version: Union[str, Literal["draft"], None] = None,
-    config: Union[str, pathlib.Path, dict, Literal["dandi"]] = "dandi",
+    config: Union[str, pathlib.Path, dict, Literal["dandi"], None] = "dandi",
     checks: Union[list, None] = None,
     ignore: Union[list[str], None] = None,
     select: Union[list[str], None] = None,
     importance_threshold: Union[str, Importance] = Importance.BEST_PRACTICE_SUGGESTION,
     skip_validate: bool = False,
-    client: Union["dandi.dandiapi.DandiAPIClient", None] = None,
+    client: Union["dandi.dandiapi.DandiAPIClient", None] = None,  # type: ignore
 ) -> Iterable[Union[InspectorMessage, None]]:
     """
     Inspect a Dandifile for common issues.
@@ -131,7 +125,7 @@ def inspect_dandi_file_path(
         The specific published version of the Dandiset to inspect.
         If None, the latest version is used.
         If there are no published versions, then 'draft' is used instead.
-    config : file path, dictionary, or "dandi", default: "dandi"
+    config : file path, dictionary, "dandi", or None, default: "dandi"
         If a file path, loads the dictionary configuration from the file.
         If a dictionary, it must be valid against the configuration schema.
         If "dandi", uses the requirements for DANDI validation.
@@ -177,14 +171,14 @@ def inspect_dandi_file_path(
         importance_threshold=importance_threshold,
         skip_validate=skip_validate,
     ):
-        message.file_path = dandi_file_path
+        message.file_path = dandi_file_path  # type: ignore
         yield message
 
 
 def inspect_url(
     *,
     url: str,
-    config: Union[str, pathlib.Path, dict, Literal["dandi"]] = "dandi",
+    config: Union[str, pathlib.Path, dict, Literal["dandi"], None] = "dandi",
     checks: Union[list, None] = None,
     ignore: Union[list[str], None] = None,
     select: Union[list[str], None] = None,
@@ -202,7 +196,7 @@ def inspect_url(
         https://dandiarchive.s3.amazonaws.com/blobs/636/57e/63657e32-ad33-4625-b664-31699b5bf664
 
         Note: this must be the `https` URL, not the 's3://' form.
-    config : file path, dictionary, or "dandi", default: "dandi"
+    config : file path, dictionary, "dandi", or None, default: "dandi"
         If a file path, loads the dictionary configuration from the file.
         If a dictionary, it must be valid against the configuration schema.
         If "dandi", uses the requirements for DANDI validation.
@@ -232,9 +226,10 @@ def inspect_url(
     filterwarnings(action="ignore", message="No cached namespaces found in .*")
     filterwarnings(action="ignore", message="Ignoring cached namespace .*")
 
-    if not isinstance(config, dict):
+    if isinstance(config, (str, pathlib.Path)):
         config = load_config(filepath_or_keyword=config)
-    validate_config(config=config)
+    if isinstance(config, dict):
+        validate_config(config=config)
 
     byte_stream = remfile.File(url=url)
     with (
@@ -249,7 +244,7 @@ def inspect_url(
                     importance=Importance.PYNWB_VALIDATION,
                     check_function_name=validation_error.name,
                     location=validation_error.location,
-                    file_path=nwbfile_path,
+                    file_path=url,
                 )
 
         nwbfile = io.read()
@@ -262,5 +257,5 @@ def inspect_url(
             select=select,
             importance_threshold=importance_threshold,
         ):
-            message.file_path = url
+            message.file_path = url  # type: ignore
             yield message