From 977dbf6402307f38f695c697d68009ddd914d237 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Fri, 14 Aug 2026 15:47:22 +1000 Subject: [PATCH] fix: ImageConstantsMixin factories silently downcast explicit dtype requests A third, distinct dtype bug from the constructor/convert() alias mismatch this branch is based on: Image.Zeros()/.Constant()/.Random()/ .Squares()/.Circles()/.Ramp()/.Sin()/.Chequerboard()/.Polygons() all build their raw pixel array with the caller's dtype, but never forward dtype= to the final Image(...) constructor call. Since dtype=None triggers the constructor's own auto-detect ("no dtype given -> any floating input becomes float32"), even a fully-correct, explicit dtype='float64' request was silently downcast to float32 -- Image.Zeros(size=5, dtype='float64').dtype was float32. Two fixes: - _resolve_pattern_options() (shared by all 9 factories above) now resolves DTYPE_ALIASES itself, uniformly regardless of whether dtype came from the caller's own argument or a default_dtype/like.dtype fallback -- Constant()'s default_dtype='float' for float values needed this too, not just the explicit-argument path. - Each factory's final constructor call now passes dtype=True ("trust the array I already built, don't second-guess it") instead of omitting dtype= entirely. Squares/Circles/Ramp/Sin/Chequerboard/ Polygons share a _pattern_image() helper -- fixed once there rather than at each of the 6 call sites. String() was already fine -- it forwards **kwargs straight to the (already-fixed) constructor rather than handling dtype itself. Extended tests/test_dtype_resolution.py with a factory x dtype matrix (10 factories x 8 dtype cases = 80 new cases) rather than one-off tests per factory. Verified against pre-fix code: 45 of the 80 fail (the cases where a factory's own default dtype doesn't coincidentally already match the auto-detect outcome), all pass with the fix. Co-Authored-By: Claude Sonnet 5 --- src/machinevisiontoolbox/ImageConstants.py | 33 ++++++++++++++---- tests/test_dtype_resolution.py | 39 ++++++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/machinevisiontoolbox/ImageConstants.py b/src/machinevisiontoolbox/ImageConstants.py index 5d99f300..b3b1b20c 100644 --- a/src/machinevisiontoolbox/ImageConstants.py +++ b/src/machinevisiontoolbox/ImageConstants.py @@ -23,7 +23,7 @@ from spatialmath.base import islistof, isscalar # from numpy.lib.arraysetops import isin -from machinevisiontoolbox.base import float_image, int_image, name2color +from machinevisiontoolbox.base import DTYPE_ALIASES, float_image, int_image, name2color from machinevisiontoolbox.base.imageio import convert, idisp, iread, iwrite from machinevisiontoolbox.Kernel import Kernel from machinevisiontoolbox.mvtb_types import Dtype @@ -110,6 +110,9 @@ def _resolve_pattern_options( else: dtype = default_dtype + if isinstance(dtype, str): + dtype = DTYPE_ALIASES.get(dtype, dtype) + if colororder is None and like is not None and like.iscolor: colororder = like.colororder_str.replace(":", "") @@ -121,7 +124,7 @@ def _pattern_image(cls, image: np.ndarray, colororder: str | None): image = np.repeat( image[..., np.newaxis], len(cls.colordict(colororder)), axis=2 ) - return cls(image, colororder=colororder) + return cls(image, colororder=colororder, dtype=True) class ImageConstantsMixin(_ImageBase if TYPE_CHECKING else object): @@ -145,6 +148,8 @@ def Zeros( :type colororder: str :param dtype: NumPy datatype, defaults to 'uint8' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param like: template image supplying default ``size`` and ``colororder`` when those are not given explicitly :type like: :class:`Image` or None, optional @@ -179,7 +184,7 @@ def Zeros( ) shape = _getshape(cls, None, None, colororder, size) - return cls(np.zeros(shape, dtype=dtype), colororder=colororder) + return cls(np.zeros(shape, dtype=dtype), colororder=colororder, dtype=True) @classmethod def Constant( @@ -202,6 +207,8 @@ def Constant( :type colororder: str :param dtype: NumPy datatype, defaults to 'uint8' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param like: template image supplying default ``size``, ``dtype`` and ``colororder`` when those are not given explicitly :type like: :class:`Image` or None, optional @@ -258,11 +265,11 @@ def Constant( planes = [] for bg in value: planes.append(np.full(shape[:2], bg, dtype=dtype)) - return cls(np.stack(planes, axis=2), colororder=colororder) + return cls(np.stack(planes, axis=2), colororder=colororder, dtype=True) else: # scalar - return cls(np.full(shape, value, dtype=dtype)) + return cls(np.full(shape, value, dtype=dtype), dtype=True) @classmethod def String( @@ -406,6 +413,8 @@ def Random( :type colororder: str :param dtype: NumPy datatype, defaults to 'uint8' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param maxval: maximum value for random values, defaults to None :type maxval: same as ``dtype``, optional :param pdf: probability density function for pixel values, defaults to None @@ -535,7 +544,7 @@ def Random( else: raise ValueError("pdf must be a 1D or 2D array") - return cls(im, colororder=colororder) + return cls(im, colororder=colororder, dtype=True) @classmethod def Squares( @@ -562,6 +571,8 @@ def Squares( :type bg: int, optional :param dtype: NumPy datatype, defaults to 'uint8' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and @@ -654,6 +665,8 @@ def Circles( :type bg: int, optional :param dtype: NumPy datatype, defaults to 'uint8' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and @@ -746,6 +759,8 @@ def Ramp( :type cycles: int, optional :param dtype: NumPy datatype, defaults to 'float32' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and @@ -842,6 +857,8 @@ def Sin( :type cycles: int, optional :param dtype: NumPy datatype, defaults to 'float32' :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and @@ -922,6 +939,8 @@ def Chequerboard( :type square: int, optional :param dtype: image data type, defaults to "uint8" :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and @@ -1012,6 +1031,8 @@ def Polygons( :type shift: int, optional :param dtype: image data type, defaults to "uint8" :type dtype: str or NumPy dtype, optional + + |dtype_aliases| :param colororder: color plane names for the output image, defaults to None :type colororder: str or None, optional :param like: template image supplying default ``size``, ``dtype`` and diff --git a/tests/test_dtype_resolution.py b/tests/test_dtype_resolution.py index 3251c17e..27728076 100644 --- a/tests/test_dtype_resolution.py +++ b/tests/test_dtype_resolution.py @@ -21,6 +21,7 @@ import numpy as np import pytest +from spatialmath import Polygon2 from machinevisiontoolbox import Image from machinevisiontoolbox.base.imageio import convert @@ -54,3 +55,41 @@ def test_image_constructor(self, dtype_in, expected): def test_convert(self, dtype_in, expected): arr = convert(np.ones((2, 3), dtype=np.uint8), dtype=dtype_in) assert arr.dtype == expected + + +# ImageConstantsMixin factory methods: a *different* bug from the above -- +# they build their raw pixel array with the (now alias-resolved) +# dtype, but historically never forwarded dtype= to the Image +# constructor call, so the constructor's own "no dtype given" auto-detect +# (any float input becomes float32) silently downcast even an explicit, +# already-correct dtype='float64' request. Fixed by passing dtype=True +# ("trust the array I already built") through _pattern_image() and each +# factory's own final constructor call. +_SQUARE = Polygon2([(2, 2), (8, 2), (8, 8), (2, 8)]) +FACTORY_CASES = [ + ("Zeros", lambda dtype: Image.Zeros(size=8, dtype=dtype)), + ("Constant_scalar", lambda dtype: Image.Constant(1.0, size=8, dtype=dtype)), + ( + "Constant_iterable", + lambda dtype: Image.Constant( + [1.0, 0.5, 0.2], size=8, colororder="RGB", dtype=dtype + ), + ), + ("Random", lambda dtype: Image.Random(size=8, dtype=dtype)), + ("Squares", lambda dtype: Image.Squares(1, size=20, dtype=dtype)), + ("Circles", lambda dtype: Image.Circles(1, size=20, dtype=dtype)), + ("Ramp", lambda dtype: Image.Ramp(size=20, dtype=dtype)), + ("Sin", lambda dtype: Image.Sin(size=20, dtype=dtype)), + ("Chequerboard", lambda dtype: Image.Chequerboard(size=20, dtype=dtype)), + ("Polygons", lambda dtype: Image.Polygons(_SQUARE, size=10, dtype=dtype)), +] +FACTORY_CASE_IDS = [c[0] for c in FACTORY_CASES] + + +@pytest.mark.parametrize("dtype_in,expected", DTYPE_CASES, ids=DTYPE_CASE_IDS) +@pytest.mark.parametrize("factory_name,factory_fn", FACTORY_CASES, ids=FACTORY_CASE_IDS) +def test_image_constants_factory_respects_explicit_dtype( + factory_name, factory_fn, dtype_in, expected +): + im = factory_fn(dtype_in) + assert im.dtype == expected