From 18c5d85d82ebd2c09a43adb278e4435658f0c2b7 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Fri, 14 Aug 2026 15:36:44 +1000 Subject: [PATCH] fix: Image(dtype='float') resolved to float64, not float32 Two independent, inconsistent dtype-string resolvers existed: convert() had its own dtype_alias dict ('float'->'float32', 'double' ->'float64', 'int'->'uint8', 'half'->'float16') since NumPy's own np.dtype('float') means float64, not float32. Image.__init__'s _infer_dtype had no such table -- it called np.dtype(dtype) raw. Since dtype is Image.__init__'s own named parameter (not forwarded via **kwargs to convert()), Image(arr, dtype='float', mono=True) sent 'float' through the unaliased path -> float64, while 'mono' went through convert()'s correctly-aliased one. Root-caused via RVC3-python's chap11.ipynb motion-detection example: VideoFile("...", mono=True, dtype='float') produced float64 frames, and arithmetic on them stayed float64 all the way to Image.disp(matplotlib=False), where OpenCV's cvtColor doesn't support CV_64F -- 'Unsupported depth of input image'. Fix: hoisted the alias table to a single shared constant, machinevisiontoolbox.base.types.DTYPE_ALIASES, used by both convert() and _infer_dtype. Documented the aliases via one Sphinx rst_epilog substitution (|dtype_aliases|, matching the existing |RVC3| pattern and bdsim's |BlockOptions| convention) referenced from both docstrings, rather than duplicating the explanation. Since every ImageSource (VideoFile, VideoCamera, ImageCollection, ...) constructs Image instances with a dtype= option, this affects all of them uniformly, not just VideoFile. Added tests/test_dtype_resolution.py: a single parametrized matrix (every DTYPE_ALIASES case x every dtype-resolving entry point) rather than scattered one-off tests per bug. Three independent, inconsistent dtype resolvers turned out to exist across this codebase (this one, plus Image.to()/.array_as()/.astype(), plus the ImageConstantsMixin factory methods -- both to be fixed on separate follow-up branches); this module is designed to grow a test method per entry point as each one is fixed, so the whole class of bug is pinned down in one place instead of drifting silently again. Co-Authored-By: Claude Sonnet 5 --- docs/source/conf.py | 1 + src/machinevisiontoolbox/ImageCore.py | 12 ++++- src/machinevisiontoolbox/base/__init__.py | 1 + src/machinevisiontoolbox/base/imageio.py | 13 ++---- src/machinevisiontoolbox/base/types.py | 14 ++++++ tests/test_dtype_resolution.py | 56 +++++++++++++++++++++++ 6 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 tests/test_dtype_resolution.py diff --git a/docs/source/conf.py b/docs/source/conf.py index 33826770..c35221e5 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -108,6 +108,7 @@ autosectionlabel_prefix_document = True rst_epilog = """ .. |RVC3| replace:: `P. Corke, Robotics, Vision & Control for Python, Springer, 2023 `__ +.. |dtype_aliases| replace:: The short-name aliases ``'int'``, ``'float'``, ``'double'``, ``'half'`` are also accepted and resolve to the Toolbox's own defaults (``uint8``, ``float32``, ``float64``, ``float16`` respectively) -- this matters because NumPy's own ``np.dtype('float')`` resolves to ``float64``, not ``float32``. See :data:`~machinevisiontoolbox.base.types.DTYPE_ALIASES`. """ # tricks to get :class:`Image` links to resolve to the correct place in the RsT docs diff --git a/src/machinevisiontoolbox/ImageCore.py b/src/machinevisiontoolbox/ImageCore.py index 7658c89a..c1860ef0 100644 --- a/src/machinevisiontoolbox/ImageCore.py +++ b/src/machinevisiontoolbox/ImageCore.py @@ -20,6 +20,7 @@ # from numpy.lib.arraysetops import isin from machinevisiontoolbox.base import ( + DTYPE_ALIASES, draw_box, draw_circle, draw_labelbox, @@ -98,8 +99,13 @@ def __init__( :type copy: bool, optional :param size: new size for the image, defaults to None :type size: tuple, optional - :param dtype: data type for image, defaults to same type as ``image`` - :type dtype: str or NumPy dtype, optional + :param dtype: data type for image; ``None`` [default] auto-detects (any + floating input becomes ``float32``; integer input becomes the + smallest unsigned/signed integer type that holds all its values); + ``True`` keeps ``image``'s own dtype as-is; ``False`` raises + ``ValueError``; otherwise a NumPy dtype string (``"uint8"``, + ``"float32"``, ...) or NumPy type. |dtype_aliases| + :type dtype: str, NumPy dtype, bool, or None, optional :param name: name of image, defaults to None :type name: str, optional :param id: numeric id of image, typically a sequence number, defaults to None @@ -381,6 +387,8 @@ def _infer_dtype(image: np.ndarray, dtype: Dtype | bool | None) -> np.dtype | No raise ValueError("bad dtype argument passed to Image constructor") else: # dtype is given, convert to a NumPy dtype + if isinstance(dtype, str): + dtype = DTYPE_ALIASES.get(dtype, dtype) try: dtype = np.dtype(dtype) except TypeError: diff --git a/src/machinevisiontoolbox/base/__init__.py b/src/machinevisiontoolbox/base/__init__.py index a6b27095..c5afbde5 100644 --- a/src/machinevisiontoolbox/base/__init__.py +++ b/src/machinevisiontoolbox/base/__init__.py @@ -56,6 +56,7 @@ # types "int_image", "float_image", + "DTYPE_ALIASES", # data "mvtb_path_to_datafile", "mvtb_load_data", diff --git a/src/machinevisiontoolbox/base/imageio.py b/src/machinevisiontoolbox/base/imageio.py index 3649fe2a..112a782b 100644 --- a/src/machinevisiontoolbox/base/imageio.py +++ b/src/machinevisiontoolbox/base/imageio.py @@ -22,7 +22,7 @@ from machinevisiontoolbox.base.color import colorspace_convert, gamma_decode from machinevisiontoolbox.base.data import mvtb_path_to_datafile -from machinevisiontoolbox.base.types import float_image, int_image +from machinevisiontoolbox.base.types import DTYPE_ALIASES, float_image, int_image try: import pyclip @@ -1176,7 +1176,7 @@ def convert( :type grey: bool or 'ITU601' [default] or 'ITU709' :param gray: synonym for ``grey`` :param dtype: a NumPy dtype string such as ``"uint8"``, ``"int16"``, ``"float32"`` or - a NumPy type like ``np.uint8``. + a NumPy type like ``np.uint8``. |dtype_aliases| :type dtype: str :param rgb: force color image to RGB order, otherwise BGR :type rgb: bool, optional @@ -1250,17 +1250,10 @@ def convert( if mono and len(image.shape) == 3: image = colorspace_convert(image, colororder, "grey") - dtype_alias = { - "int": "uint8", - "float": "float32", - "double": "float64", - "half": "float16", - } - if dtype is not None: # default types if isinstance(dtype, str): - dtype = dtype_alias.get(dtype, dtype) + dtype = DTYPE_ALIASES.get(dtype, dtype) if "int" in str(dtype): image = int_image(image, intclass=dtype, maxintval=maxintval) diff --git a/src/machinevisiontoolbox/base/types.py b/src/machinevisiontoolbox/base/types.py index 2c847ed5..5c32a6a2 100644 --- a/src/machinevisiontoolbox/base/types.py +++ b/src/machinevisiontoolbox/base/types.py @@ -8,6 +8,20 @@ from machinevisiontoolbox.mvtb_types import Dtype +#: Short-name aliases for dtype strings accepted throughout the Toolbox +#: (e.g. Image(..., dtype='float'), VideoFile(..., dtype='float')). +#: NumPy's own np.dtype('float') resolves to float64, not float32 -- these +#: aliases exist specifically so 'float'/'int' mean the Toolbox's own +#: default single-precision/8-bit types, not NumPy's C-double/C-long +#: defaults. Single source of truth: resolve any dtype string through this +#: table (dtype_alias.get(dtype, dtype)) before passing it to np.dtype(). +DTYPE_ALIASES: dict[str, str] = { + "int": "uint8", + "float": "float32", + "double": "float64", + "half": "float16", +} + def int_image( image: np.ndarray, intclass: Dtype = "uint8", maxintval: int | None = None diff --git a/tests/test_dtype_resolution.py b/tests/test_dtype_resolution.py new file mode 100644 index 00000000..3251c17e --- /dev/null +++ b/tests/test_dtype_resolution.py @@ -0,0 +1,56 @@ +""" +Consolidated dtype-resolution consistency tests. + +Multiple entry points resolve a user-supplied dtype string into an actual +NumPy dtype: the Image constructor (via _infer_dtype), convert(), and (once +fixed on their own branches) Image.to()/.array_as()/.astype() and the +ImageConstantsMixin factory methods (Zeros, Constant, Random, ...). All of +them are supposed to honour the same short-name aliases (DTYPE_ALIASES: +'int'->uint8, 'float'->float32, 'double'->float64, 'half'->float16) plus +pass explicit NumPy dtype strings through unchanged. + +Three independent, inconsistent implementations of this resolution existed +at once (found 2026-08 while working through RVC3-python's chap11.ipynb -- +VideoFile(..., mono=True, dtype='float') produced float64 frames instead of +float32, because Image.__init__'s own dtype resolution didn't share +convert()'s alias table). This module pins every entry point against the +same shared matrix of cases so that kind of drift can't happen silently +again -- add a test method to the relevant class below rather than a new +one-off test elsewhere when another entry point is fixed. +""" + +import numpy as np +import pytest + +from machinevisiontoolbox import Image +from machinevisiontoolbox.base.imageio import convert + +# (dtype spec passed in, expected resolved np.dtype) +DTYPE_CASES = [ + # short-name aliases (DTYPE_ALIASES) -- NumPy's own np.dtype(...) would + # resolve 'float' to float64 and 'int' to platform int, not these. + ("int", np.dtype("uint8")), + ("float", np.dtype("float32")), + ("double", np.dtype("float64")), + ("half", np.dtype("float16")), + # explicit NumPy dtype strings, not in DTYPE_ALIASES -- must pass + # through unchanged, not be (mis)matched against an alias. + ("uint8", np.dtype("uint8")), + ("int16", np.dtype("int16")), + ("float32", np.dtype("float32")), + ("float64", np.dtype("float64")), +] +DTYPE_CASE_IDS = [c[0] for c in DTYPE_CASES] + + +@pytest.mark.parametrize("dtype_in,expected", DTYPE_CASES, ids=DTYPE_CASE_IDS) +class TestDtypeResolutionConsistency: + """Every entry point below must resolve the same dtype_in the same way.""" + + def test_image_constructor(self, dtype_in, expected): + im = Image(np.ones((2, 3), dtype=np.uint8), dtype=dtype_in) + assert im.dtype == expected + + def test_convert(self, dtype_in, expected): + arr = convert(np.ones((2, 3), dtype=np.uint8), dtype=dtype_in) + assert arr.dtype == expected