From c5b6aebdaa42003bc48d0428d3381599f325d1df Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Thu, 30 Jul 2026 11:16:42 +1000 Subject: [PATCH 1/3] docs: log pgraph-python CI gap and conda-migration decision to tech-debt pgraph-python is a declared pyproject.toml dependency but is entirely absent from ci.yml's create-args -- BundleAdjust.py's only import of it is wrapped in a bare except that silently falls back, so CI has been exercising that code in "not installed" mode the whole time. Also records the decision not to do the conda->pip migration now: known non-technical origin (contributor's conda preference), and today's new branch protection (required All tests passed + build, strict mode) already contains the blast radius of a future incident the way it hit multiple PRs at once today. --- tech-debt.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tech-debt.md b/tech-debt.md index c9fb0440..9180d450 100644 --- a/tech-debt.md +++ b/tech-debt.md @@ -147,6 +147,33 @@ new extra or dependency. Patched directly (added `ipython`/`pygments` to `create-args`) and merged 2026-07-29 — the underlying architectural gap below is still open. +**Third example, found 2026-07-29 (pre-existing, not a new incident)**: +`pgraph-python` is a real, declared dependency in `pyproject.toml`'s +`dependencies` list, but is completely absent from `ci.yml`'s +`create-args` — not merely unpinned, never installed at all. The only +file that imports it, `BundleAdjust.py`, does so inside a bare +`except:` (see the bare-except finding elsewhere in this file) that +silently falls back when the import fails. So every CI run has been +exercising `BundleAdjust`'s pgraph-dependent code in "not installed" +fallback mode this whole time, with nothing surfacing it. Not fixed +here — decided (2026-07-29) to leave the conda→pip migration itself as +deferred tech debt rather than keep patching individual missing +packages one at a time; noting this one so it's not lost. + +**Decision, 2026-07-29**: known who introduced the conda/micromamba +setup and why (a well-intentioned contributor's preference for conda, +not a technical requirement) — confirmed not urgent enough to fix now. +Branch protection on `main` (`All tests passed` + `build` required, +strict/up-to-date-with-base required) now contains the *blast radius* +of a future incident like this — it'll block just the one PR that hits +it, rather than silently landing on `main` and confusing every other +open PR at once, which was the actually painful part of the opencv5 +incident. The root architectural gap remains real and will very likely +surface again the same way (a fourth missing/drifted package), but +that's an acceptable trade for now given the fix requires careful +cross-OS testing (libegl/Xvfb handling, `matplotlib-base` vs +`matplotlib` naming) rather than a quick patch. + ### Fix Normalize to the same `actions/setup-python` + `pip install .[dev]` From 1b74ae8efc240e43c338dec8415f7198de2a0d3a Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Thu, 30 Jul 2026 12:02:50 +1000 Subject: [PATCH 2/3] docs: log mypy-not-in-CI as high-priority tech debt, with fresh audit Prompted by fixing the _ImageBase Protocol gaps in this same branch: if a mixin calls a cross-mixin self.attr not yet declared in _image_typing.py, nothing catches it -- mypy isn't in pyproject.toml's dev extra or any CI workflow. Same "hand-maintained shadow list, no automated check" pattern as the docs sidebar and ci.yml create-args issues found earlier today, just quieter (no visible failure mode at all, vs. a missing page or a red build). Ran mypy fresh (524 errors/31 files, superseding the stale April NOTES audit) and categorized. Corrects an initial hypothesis: checked whether attr-defined errors trace to _ImageBase gaps -- they don't, currently. The two real dominant causes are machinevisiontoolbox/base/__init__.py's unresolvable wildcard re-exports and cv2 dynamic dispatch, both unrelated to the mixin-Protocol question that prompted the audit. --- tech-debt.md | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tech-debt.md b/tech-debt.md index 9180d450..93975529 100644 --- a/tech-debt.md +++ b/tech-debt.md @@ -1,5 +1,100 @@ # Technical Debt +## [HIGH PRIORITY] mypy is not run anywhere in CI or dev tooling + +Found 2026-07-29 while fixing the `_ImageBase` Protocol gaps in +`ImageBlobs.py`/`ImageRegionFeatures.py`/`ImagePointFeatures.py` +(see git history — three mixins were missing the +`class XMixin(_ImageBase if TYPE_CHECKING else object)` pattern every +other mixin uses). That fix prompted the question: if a new method is +added to a mixin and it's accessed via `self.` from a *different* +mixin, and it's not yet declared in `_image_typing.py`'s `_ImageBase` +Protocol, what actually catches that? Answer: **nothing, currently**. +`mypy` is not in `pyproject.toml`'s `dev` extra and does not run in any +`.github/workflows/*.yml` — confirmed by grep, zero hits. So a Protocol +gap like this doesn't fail a build or even show a warning; it just +silently produces incomplete/wrong type information for anyone using an +editor with type-checking (Pylance, mypy in an IDE), with no automated +signal anywhere. This is the quietest version of a pattern that's +already bitten this project twice today in more visible forms (the +`image_class.rst` autosummary list silently going stale for the whole +`Image` sidebar, and `ci.yml`'s conda `create-args` list silently +missing `pgraph-python`/drifting on `opencv`) — a hand-maintained +shadow list with no automated check that it stays in sync with reality. + +**Ran `mypy src/machinevisiontoolbox --ignore-missing-imports` fresh, +2026-07-29** (superseding the stale, less-categorized April audit in +`NOTES`): **524 errors in 31 files** (checked 49 source files). By +category: + +| Code | Count | +|---|---| +| `attr-defined` | 84 | +| `assignment` | 82 | +| `union-attr` | 76 | +| `index` | 75 | +| `arg-type` | 53 | +| `misc` | 26 | +| `name-defined` | 24 | +| `var-annotated` | 18 | +| `valid-type` | 15 | +| `operator` | 15 | +| `call-overload` | 14 | +| `return-value` | 13 | +| `has-type` | 11 | +| `return` | 6 | +| `no-redef` | 5 | +| `override` | 4 | +| `method-assign` | 2 | +| `call-arg` | 1 | + +**Correcting an initial hypothesis**: expected most `attr-defined` +errors to trace to incomplete `_ImageBase` coverage (only ~96 of +`Image`'s ~290 public members are declared — by design, since the +Protocol only needs to cover attributes actually cross-referenced +between mixins, not the full public API). Checked the real breakdown +instead of assuming: **none** of the current `attr-defined` errors are +actually `_ImageBase` gaps. The two real dominant causes are unrelated: +- 23 of 84: `machinevisiontoolbox/base/__init__.py` re-exports every + submodule via wildcard `from X import *` (9 submodules) with no + explicit `__all__`/direct re-export list; mypy can't reliably resolve + names through that chain, so every file that does + `from machinevisiontoolbox.base import (draw_circle, plot_labelbox, + findpeaks2d, ...)` gets a false "module has no attribute" even though + these work fine at runtime. +- 8 of 84: `cv2._create` dynamic dispatch (`getattr(cv2, ...)` / + `getattr(cv2.xfeatures2d, ...)` patterns in `ImagePointFeatures.py`'s + feature-detector dict) — mypy can't type-check dynamic attribute + access, expected and low-value to fix. +- The remaining ~53 are scattered; `VisualServo.py` alone accounts for + 44 of the 84 `attr-defined` errors (a mix of real typos like + `"plotpose"; maybe "plot_pose"?` and missing `machinevisiontoolbox.base` + attributes via the same wildcard-import issue) and is worth its own + look independent of the mixin-Protocol question that prompted this + audit. + +The `_ImageBase`-completeness risk described above is still real, just +currently *latent* rather than demonstrated by a live error — worth +re-running this same `mypy` audit after any future mixin refactor to +catch it if it does start manifesting. + +### Fix + +Two independent pieces, roughly in priority order: +1. Fix `machinevisiontoolbox/base/__init__.py`'s wildcard re-exports + (add explicit `__all__` composed from each submodule's own `__all__`, + or switch to explicit `from X import (name1, name2, ...)` — either + should immediately clear ~23+ of the `attr-defined` count and is a + mechanical, low-risk change). +2. Wire `mypy` into CI (even just as a non-blocking/advisory job at + first, given 524 existing errors) so future drift is visible instead + of silent. Add `mypy` to `pyproject.toml`'s `dev` extra either way. +Do not attempt to fix all 524 errors in one pass — triage by category +(the `[call-arg]`/`[override]`/`[return]` categories are more likely to +be real bugs; `assignment`/`arg-type`/`index` are more likely the +`ArrayLike`-union-too-broad pattern the April `NOTES` audit already +identified). + ## `Image.ncdf` is documented as deprecated but never warns `ncdf` (`src/machinevisiontoolbox/ImageWholeFeatures.py:511-523`) has a From 0d31fc52f881df6cfda35c5ab11e4f9753e65aa0 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Thu, 30 Jul 2026 16:22:58 +1000 Subject: [PATCH 3/3] fix: bring 3 mixins in line with the TYPE_CHECKING _ImageBase pattern Every mixin in src/machinevisiontoolbox/ follows class XMixin(_ImageBase if TYPE_CHECKING else object), with _ImageBase imported inside an if TYPE_CHECKING: block -- _ImageBase is a pure Protocol (all ... stubs, no implementation), so this is a zero-runtime- behavior-change pattern used purely to give type checkers attribute info without adding anything to the real MRO. Three files didn't follow it: - ImageBlobsMixin / ImageRegionFeaturesMixin: no _ImageBase at all (plain `object` base), so self._A/self.colororder/etc. accesses inside these two mixins had no type info. - ImagePointFeaturesMixin: the opposite problem -- imported and inherited _ImageBase unconditionally, putting a Protocol class into the real runtime MRO. Verified this was always a no-op in practice (_image_typing.py's own docstring: "never instantiated, adds nothing to the MRO"; confirmed no other code in the file referenced _ImageBase outside the import and class declaration), so making it TYPE_CHECKING-only is a safe, behavior-preserving fix. Verified: `cls.__bases__` is `(object,)` for all three mixins now (matching every other mixin), full suite passes (816 passed, 15 skipped). --- src/machinevisiontoolbox/ImageBlobs.py | 7 +++++-- src/machinevisiontoolbox/ImagePointFeatures.py | 8 +++++--- src/machinevisiontoolbox/ImageRegionFeatures.py | 7 +++++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/machinevisiontoolbox/ImageBlobs.py b/src/machinevisiontoolbox/ImageBlobs.py index 4e342929..9deb4f86 100644 --- a/src/machinevisiontoolbox/ImageBlobs.py +++ b/src/machinevisiontoolbox/ImageBlobs.py @@ -11,7 +11,7 @@ import webbrowser from collections import UserList, namedtuple from dataclasses import dataclass -from typing import Any, cast +from typing import TYPE_CHECKING, Any, cast import cv2 import matplotlib.pyplot as plt @@ -24,6 +24,9 @@ from machinevisiontoolbox.base import plot_labelbox, mpl_styling from machinevisiontoolbox.decorators import array_result, scalar_result +if TYPE_CHECKING: + from machinevisiontoolbox._image_typing import _ImageBase + """ NOTES @@ -2623,7 +2626,7 @@ def dotfile( dot_stream.close() -class ImageBlobsMixin: +class ImageBlobsMixin(_ImageBase if TYPE_CHECKING else object): def blobs(self, **kwargs) -> Blobs: """ Find and describe blobs in image diff --git a/src/machinevisiontoolbox/ImagePointFeatures.py b/src/machinevisiontoolbox/ImagePointFeatures.py index d185ad03..7489013f 100644 --- a/src/machinevisiontoolbox/ImagePointFeatures.py +++ b/src/machinevisiontoolbox/ImagePointFeatures.py @@ -3,7 +3,7 @@ """ import math -from typing import Any, Iterator +from typing import TYPE_CHECKING, Any, Iterator import cv2 import matplotlib.lines as mlines @@ -21,13 +21,15 @@ name2color, safe_plt_show, ) -from machinevisiontoolbox._image_typing import _ImageBase from machinevisiontoolbox.decorators import ( array_result, array_result2, scalar_result, ) +if TYPE_CHECKING: + from machinevisiontoolbox._image_typing import _ImageBase + # TODO, either subclass SIFTFeature(BaseFeature2D) or just use BaseFeature2D # directly @@ -2012,7 +2014,7 @@ class LUCIDFeature(BaseFeature2D): pass -class ImagePointFeaturesMixin(_ImageBase): +class ImagePointFeaturesMixin(_ImageBase if TYPE_CHECKING else object): def _image2feature( self, cls, diff --git a/src/machinevisiontoolbox/ImageRegionFeatures.py b/src/machinevisiontoolbox/ImageRegionFeatures.py index 32620264..9d160f54 100644 --- a/src/machinevisiontoolbox/ImageRegionFeatures.py +++ b/src/machinevisiontoolbox/ImageRegionFeatures.py @@ -9,7 +9,7 @@ import numpy as np from ansitable import ANSITable, Column from spatialmath import SE3 -from typing import Any +from typing import TYPE_CHECKING, Any from machinevisiontoolbox.decorators import array_result from machinevisiontoolbox.base import plot_labelbox @@ -22,8 +22,11 @@ _pytesseract = None _pytesseract_available = False +if TYPE_CHECKING: + from machinevisiontoolbox._image_typing import _ImageBase -class ImageRegionFeaturesMixin: + +class ImageRegionFeaturesMixin(_ImageBase if TYPE_CHECKING else object): def MSER(self, **kwargs: Any) -> "MSERFeature": """ Find MSER features in image