MyPy Compliance (#1300)

* Initial config. * Better config. * Various MyPy fixes. * More MyPy fixes. * More MyPy fixes. * Finalised MyPy. * Pre-commit fixes. * Test fixes. * Address third-party MyPy problems. * Better handling of rgb parameter. * Change log entry. * Remove overzealous rgb None checking. * Better changelog entry. * Improve PathLike type alias. * Don't bother setting band to None if rgb. * Remove unnecessary assertion. * Alter pyproject.toml format. * Nicer type narrowing. * TYPE_CHECKING Sequence import and correct use of BBOX_C in docstring parameters. --------- Co-authored-by: Bill Little <[email protected]>
bjlittle · Jan 31, 2025 · 8032f67 · 8032f67
1 parent 6f86f35
commit 8032f67
Show file tree

Hide file tree

Showing 19 changed files with 203 additions and 132 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -35,6 +35,13 @@ repos:
         types_or: [python, markdown, rst]
         additional_dependencies: [tomli]
 
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: 'v1.14.1'
+    hooks:
+      - id: mypy
+        # https://github.com/python/mypy/issues/13916
+        pass_filenames: false
+
   - repo: https://github.com/numpy/numpydoc
     rev: v1.8.0
     hooks:

diff --git a/changelog/1300.enhancement.rst b/changelog/1300.enhancement.rst
@@ -0,0 +1,3 @@
+Made the entire code base `mypy <https://github.com/python/mypy>`__ compliant.
+This should make development faster and more intuitive for anyone using an IDE
+and/or any sort of type-checking.
diff --git a/pyproject.toml b/pyproject.toml
@@ -58,6 +58,26 @@ ignore-words-list = "whet,projets"
 skip = ".git,*.css,*.ipynb,*.js,*.html,*.map,*.po,CODE_OF_CONDUCT.md"
 
 
+[tool.mypy]
+enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]
+exclude = []
+files = ["src/geovista"]
+strict = true
+warn_unreachable = true
+
+
+[[tool.mypy.overrides]]
+# Problem caused by the click module - out of our control.
+disallow_untyped_decorators = false
+module = "geovista.cli"
+
+
+[[tool.mypy.overrides]]
+# Subclassing third-party classes - out of our control.
+disallow_subclassing_any = false
+module = ["geovista.geoplotter", "geovista.qt", "geovista.report"]
+
+
 [tool.numpydoc_validation]
 checks = [
   "all", # Enable all numpydoc validation rules, apart from the following:
@@ -123,10 +143,6 @@ xfail_strict = "True"
 
 [tool.repo-review]
 ignore = [
-  # https://learn.scientific-python.org/development/guides/style/#MY100
-  "MY100", # Uses MyPy (pyproject config).
-  # https://learn.scientific-python.org/development/guides/style/#PC140
-  "PC140", # Uses mypy.
   # https://learn.scientific-python.org/development/guides/style/#PC180
   "PC180", # Uses prettier.
 ]

diff --git a/src/geovista/__init__.pyi b/src/geovista/__init__.pyi
@@ -10,8 +10,15 @@ from .pantry.textures import (
 )
 from .report import Report
 
+
+__version__: str
+GEOVISTA_IMAGE_TESTING: bool
+
+
 __all__ = [
+    "__version__",
     "GeoPlotter",
+    "GEOVISTA_IMAGE_TESTING",
     "Report",
     "Transform",
     "black_marble",

diff --git a/src/geovista/bridge.py b/src/geovista/bridge.py
@@ -19,7 +19,7 @@
 
 from __future__ import annotations
 
-from pathlib import Path, PurePath
+from pathlib import Path
 from typing import TYPE_CHECKING, TypeAlias
 import warnings
 
@@ -61,10 +61,10 @@
 ]
 
 # type aliases
-PathLike: TypeAlias = str | PurePath
+PathLike: TypeAlias = str | Path
 """Type alias for an asset file path."""
 
-Shape: TypeAlias = tuple[int]
+Shape: TypeAlias = tuple[int, ...]
 """Type alias for a tuple of integers."""
 
 # constants
@@ -573,6 +573,7 @@ def from_2d(
         cls._verify_2d(xs, ys)
         shape, ndim = xs.shape, xs.ndim
 
+        points_shape: tuple[int, int]
         if ndim == 2:
             # we have shape (M+1, N+1)
             rows, cols = points_shape = shape
@@ -702,8 +703,6 @@ def from_points(
 
             if not name:
                 name = NAME_POINTS
-            if not isinstance(name, str):
-                name = str(name)
 
             mesh.field_data[GV_FIELD_NAME] = np.array([name])
             mesh[name] = data
@@ -719,7 +718,7 @@ def from_tiff(
         cls,
         fname: PathLike,
         name: str | None = None,
-        band: int | None = 1,
+        band: int = 1,
         rgb: bool | None = False,
         sieve: bool | None = False,
         size: int | None = None,
@@ -743,9 +742,9 @@ def from_tiff(
             Defaults to :data:`NAME_POINTS`. Note that, ``{units}`` may be
             used as a placeholder for the units of the data array e.g.,
             ``"Elevation / {units}"``.
-        band : int, optional
+        band : int, default=1
             The band index to read from the GeoTIFF. Note that, the `band`
-            index is one-based. Defaults to the first band i.e., ``band=1``.
+            index is one-based.
         rgb : bool, default=False
             Specify whether to read the GeoTIFF as an ``RGB`` or ``RGBA`` image.
             When ``rgb=True``, the `band` index is ignored.
@@ -820,7 +819,6 @@ def from_tiff(
             fname = Path(fname)
 
         fname = fname.resolve(strict=True)
-        band = None if rgb else band
 
         if size is None:
             size = RIO_SIEVE_SIZE
@@ -926,7 +924,7 @@ def from_unstructured(
         start_index: int | None = None,
         name: str | None = None,
         crs: CRSLike | None = None,
-        rgb: bool | None = None,
+        rgb: bool | None = False,
         radius: float | None = None,
         zlevel: int | None = None,
         zscale: float | None = None,
@@ -1006,6 +1004,9 @@ def from_unstructured(
         .. versionadded:: 0.1.0
 
         """
+        if rgb is None:
+            rgb = False
+
         xs, ys = np.asanyarray(xs), np.asanyarray(ys)
         shape = xs.shape
 
@@ -1154,8 +1155,6 @@ def from_unstructured(
             if not name:
                 size = data.size // data.shape[-1] if rgb else data.size
                 name = NAME_POINTS if size == mesh.n_points else NAME_CELLS
-            if not isinstance(name, str):
-                name = str(name)
 
             mesh.field_data[GV_FIELD_NAME] = np.array([name])
             mesh[name] = data

diff --git a/src/geovista/cache/__init__.py b/src/geovista/cache/__init__.py
@@ -16,7 +16,7 @@
 from functools import wraps
 import os
 from pathlib import Path
-from typing import IO, TYPE_CHECKING, TypeAlias
+from typing import IO, TYPE_CHECKING, Any, AnyStr, TypeAlias
 
 import pooch
 
@@ -38,7 +38,7 @@
     from collections.abc import Callable
 
 # type aliases
-FileLike: TypeAlias = str | IO
+FileLike: TypeAlias = str | IO[AnyStr]
 """Type alias for filename or file-like object."""
 
 BASE_URL: str = "https://github.com/bjlittle/geovista-data/raw/{version}/assets/"
@@ -94,10 +94,13 @@
 
 
 @wraps(CACHE._fetch)  # noqa: SLF001
-def _fetch(*args: str, **kwargs: bool | Callable) -> str:  # numpydoc ignore=GL08
+def _fetch(
+    *args: str, **kwargs: bool | Callable[..., Any]
+) -> str:  # numpydoc ignore=GL08
     # default to our http/s downloader with user-agent headers
     kwargs.setdefault("downloader", _downloader)
-    return CACHE._fetch(*args, **kwargs)  # noqa: SLF001
+    result: str = CACHE._fetch(*args, **kwargs)  # noqa: SLF001
+    return result
 
 
 # override the original Pooch.fetch method with our
@@ -107,7 +110,7 @@ def _fetch(*args: str, **kwargs: bool | Callable) -> str:  # numpydoc ignore=GL0
 
 def _downloader(
     url: str,
-    output_file: FileLike,
+    output_file: FileLike[Any],
     poocher: pooch.Pooch,
     check_only: bool | None = False,
 ) -> bool | None:
@@ -144,11 +147,11 @@ def _downloader(
 
     # see https://github.com/readthedocs/readthedocs.org/issues/11763
     headers = {"User-Agent": f"geovista ({geovista.__version__})"}
-    downloader = pooch.HTTPDownloader(headers=headers)
+    downloader: Callable[..., bool | None] = pooch.HTTPDownloader(headers=headers)
     return downloader(url, output_file, poocher, check_only=check_only)
 
 
-def pooch_mute(silent: bool | None = True) -> bool:
+def pooch_mute(silent: bool | None = None) -> bool:
     """Control the :mod:`pooch` cache manager logger verbosity.
 
     Updates the status variable :data:`GEOVISTA_POOCH_MUTE`.
@@ -171,6 +174,9 @@ def pooch_mute(silent: bool | None = True) -> bool:
     """
     global GEOVISTA_POOCH_MUTE  # noqa: PLW0603
 
+    if silent is None:
+        silent = True
+
     level = "WARNING" if silent else "NOTSET"
     pooch.utils.get_logger().setLevel(level)
     original = GEOVISTA_POOCH_MUTE
@@ -195,10 +201,10 @@ def reload_registry(fname: str | None = None) -> None:
 
     """
     if fname is None:
-        fname = (Path(__file__).parent / "registry.txt").open(
+        text_io = (Path(__file__).parent / "registry.txt").open(
             "r", encoding="utf-8", errors="strict"
         )
-    CACHE.load_registry(fname)
+    CACHE.load_registry(text_io)
 
 
 # configure the pooch cache manager logger verbosity

diff --git a/src/geovista/cli.py b/src/geovista/cli.py
@@ -85,23 +85,26 @@ def _download_group(
     if fg_colour is None:
         fg_colour = FG_COLOUR
 
-    name: str = "" if name is None else f"{name} "
+    name_post: str = "" if name is None else f"{name} "
 
     n_fnames: int = len(fnames)
     width: int = len(str(n_fnames))
 
     previous = pooch_mute(silent=True)
 
-    click.echo(f"Downloading {n_fnames} {name}registered asset{_plural(n_fnames)}:")
+    click.echo(
+        f"Downloading {n_fnames} {name_post}registered asset{_plural(n_fnames)}:"
+    )
     for i, fname in enumerate(fnames):
         click.echo(f"[{i + 1:0{width}d}/{n_fnames}] Downloading ", nl=False)
         click.secho(f"{fname} ", nl=False, fg=fg_colour)
         click.echo("... ", nl=False)
         processor = None
-        name = pathlib.Path(fname)
-        if decompress and (suffix := name.suffix) in pooch.Decompress.extensions:
-            name = name.stem.removesuffix(suffix)
-            processor = pooch.Decompress(method="auto", name=name)
+        name_path = pathlib.Path(fname)
+        if decompress and (suffix := name_path.suffix) in pooch.Decompress.extensions:
+            processor = pooch.Decompress(
+                method="auto", name=name_path.stem.removesuffix(suffix)
+            )
         CACHE.fetch(fname, processor=processor)
         click.secho("done!", fg="green")
 
@@ -465,10 +468,10 @@ def examples(
 
     if groups:
         click.echo("Available example groups:")
-        groups = _groups()
-        n_groups = len(groups)
+        groups_new = _groups()
+        n_groups = len(groups_new)
         width = len(str(n_groups))
-        for i, group in enumerate(groups):
+        for i, group in enumerate(groups_new):
             click.echo(f"[{i + 1:0{width}d}/{n_groups}] ", nl=False)
             click.secho(f"{group}", fg="green")
         click.echo("\n👍 All done!")
@@ -491,9 +494,9 @@ def examples(
         return
 
     if run_group:
-        group = [script for script in EXAMPLES[1:] if script.startswith(run_group)]
-        n_group = len(group)
-        for i, script in enumerate(group):
+        filtered = [script for script in EXAMPLES[1:] if script.startswith(run_group)]
+        n_group = len(filtered)
+        for i, script in enumerate(filtered):
             msg = f"Running {run_group!r} example {script!r} ({i + 1} of {n_group}) ..."
             click.secho(msg, fg="green")
             module = importlib.import_module(f"geovista.examples.{script}")