Merge pull request #485 from PyAutoLabs/feature/mask1d-shape-native-scalar-widening

fix: widen scalar pixel_scales / shape_native at the two sites #464 missed

Author: Jammy2211

autoarray/geometry/geometry_util.py

@@ -7,25 +7,34 @@
 
 def convert_shape_native_1d(shape_native: Union[int, Tuple[int]]) -> Tuple[int]:
     """
-    Convert an input `shape_native` of type `int` to a tuple `(int,)`. If the input is already a
-    `(int,)` tuple it is returned unchanged.
+    Convert an input `shape_native` given as a single integer scalar to a tuple `(int,)`. If
+    the input is already a `(int,)` tuple it is returned unchanged.
 
     This enables users to input `shape_native` as a single integer value and have the type
     automatically normalised to `(int,)` which is used internally by 1D data structures.
 
+    Any concrete integer scalar is widened — `int` and `np.integer` — not just an exact `int`.
+    An `np.integer` is what indexing a shape tuple or reading a FITS header returns, so it
+    reaches this function on paths a user would consider ordinary. The widened value is cast
+    to a Python `int`, so the tuple this returns is always `(int,)` regardless of what went in.
+
+    A `float` is deliberately *not* widened: `shape_native` counts pixels, so a non-integer is
+    a mistake worth surfacing rather than one to normalise away. Neither is a `bool`, nor a JAX
+    tracer — see :func:`autoarray.validate.is_concrete_integer`.
+
     Parameters
     ----------
     shape_native
-        The 1D shape to convert, either as a plain `int` or a 1-element tuple `(int,)`.
+        The 1D shape to convert, either as a plain integer scalar or a 1-element tuple `(int,)`.
 
     Returns
     -------
     Tuple[int]
         The shape as a 1-element tuple `(int,)`.
     """
 
-    if type(shape_native) is int:
-        shape_native = (shape_native,)
+    if validate.is_concrete_integer(shape_native):
+        shape_native = (int(shape_native),)
 
     return shape_native
 

autoarray/mask/mask_1d.py

@@ -10,6 +10,7 @@
 
 from autoarray.mask.derive.grid_1d import DeriveGrid1D
 from autoarray.mask.derive.mask_1d import DeriveMask1D
+from autoarray.geometry import geometry_util
 from autoarray.geometry.geometry_1d import Geometry1D
 from autoarray.structures.abstract_structure import Structure
 from autoarray.structures.arrays import array_1d_util
@@ -68,8 +69,7 @@ def __init__(
         if invert:
             mask = ~mask
 
-        if type(pixel_scales) is float:
-            pixel_scales = (pixel_scales,)
+        pixel_scales = geometry_util.convert_pixel_scales_1d(pixel_scales=pixel_scales)
 
         if len(mask.shape) != 1:
             raise exc.MaskException("The input mask is not a one dimensional array")

autoarray/validate.py

@@ -70,6 +70,29 @@ def is_concrete_scalar(value: Any) -> bool:
     return isinstance(value, (int, float, np.integer, np.floating))
 
 
+def is_concrete_integer(value: Any) -> bool:
+    """
+    Returns ``True`` if ``value`` is a concrete Python or NumPy **integer** scalar.
+
+    The integer-only counterpart of :func:`is_concrete_scalar`, for parameters which
+    count pixels rather than measure them — a ``shape_native`` of ``5.0`` is a mistake
+    worth surfacing, not a value to silently widen, so a ``float`` returns ``False``
+    here where ``is_concrete_scalar`` would accept it.
+
+    Tracer-safety and the ``bool`` exclusion carry over from
+    :func:`is_concrete_scalar` unchanged.
+
+    Parameters
+    ----------
+    value
+        The value to test.
+    """
+    if isinstance(value, bool):
+        return False
+
+    return isinstance(value, (int, np.integer))
+
+
 def _raise(
     name: str,
     rule: str,

test_autoarray/geometry/test_geometry_util.py

@@ -21,18 +21,25 @@ def test__convert_pixel_scales_1d__widens_any_real_scalar(pixel_scales):
     """
     Any concrete real scalar widens, not just an exact `float`. `int` is what a user types by
     hand; `np.floating` is what indexing an array or reading a FITS header returns.
+
+    The tuple-ness is asserted before the value: `np.float64(1.0) == (1.0,)` NumPy-broadcasts
+    to `array([True])`, which is truthy, so a value-only assertion passes on the unwidened
+    scalar and tests nothing.
     """
-    assert aa.util.geometry.convert_pixel_scales_1d(pixel_scales=pixel_scales) == (1.0,)
+    pixel_scales = aa.util.geometry.convert_pixel_scales_1d(pixel_scales=pixel_scales)
+
+    assert type(pixel_scales) is tuple
+    assert pixel_scales == (1.0,)
 
 
 @pytest.mark.parametrize(
     "pixel_scales", [1, 1.0, np.float64(1.0), np.int32(1), np.float32(1.0)]
 )
 def test__convert_pixel_scales_2d__widens_any_real_scalar(pixel_scales):
-    assert aa.util.geometry.convert_pixel_scales_2d(pixel_scales=pixel_scales) == (
-        1.0,
-        1.0,
-    )
+    pixel_scales = aa.util.geometry.convert_pixel_scales_2d(pixel_scales=pixel_scales)
+
+    assert type(pixel_scales) is tuple
+    assert pixel_scales == (1.0, 1.0)
 
 
 @pytest.mark.parametrize("pixel_scales", [1, np.float64(1.0), np.int32(1)])
@@ -86,6 +93,64 @@ def test__convert_pixel_scales__a_bool_is_not_treated_as_a_scalar():
     assert aa.util.geometry.convert_pixel_scales_2d(pixel_scales=True) is True
 
 
+@pytest.mark.parametrize("shape_native", [5, np.int32(5), np.int64(5)])
+def test__convert_shape_native_1d__widens_any_integer_scalar(shape_native):
+    """
+    Any concrete integer scalar widens, not just an exact `int`. An `np.integer` is what
+    indexing a shape tuple or reading a FITS header returns, and `Array1D.full` then does
+    `shape_native[0]` on whatever comes back — a bare scalar raised `IndexError` there.
+
+    The tuple-ness is asserted before the value: `np.int32(5) == (5,)` NumPy-broadcasts to
+    `array([True])`, which is truthy, so a value-only assertion passes on the unwidened
+    scalar and tests nothing.
+    """
+    shape_native = aa.util.geometry.convert_shape_native_1d(shape_native=shape_native)
+
+    assert type(shape_native) is tuple
+    assert shape_native == (5,)
+
+
+@pytest.mark.parametrize("shape_native", [5, np.int32(5), np.int64(5)])
+def test__convert_shape_native_1d__widened_entry_is_a_python_int(shape_native):
+    """
+    The widened value is cast, so a NumPy scalar never reaches the shape stored on a
+    structure. `5 == np.int32(5)` in Python, so the cast has to be asserted on the type.
+    """
+    (entry,) = aa.util.geometry.convert_shape_native_1d(shape_native=shape_native)
+    assert type(entry) is int
+
+
+def test__convert_shape_native_1d__a_float_is_not_widened():
+    """
+    Unlike `pixel_scales`, `shape_native` counts pixels rather than measuring them, so a
+    `float` is a mistake worth surfacing rather than one to normalise away — the predicate
+    here is `is_concrete_integer`, not `is_concrete_scalar`.
+    """
+    assert aa.util.geometry.convert_shape_native_1d(shape_native=5.0) == 5.0
+
+
+def test__convert_shape_native_1d__a_bool_is_not_treated_as_a_scalar():
+    """`bool` is a subclass of `int`, but `True` reaching a pixel count is a different mistake."""
+    assert aa.util.geometry.convert_shape_native_1d(shape_native=True) is True
+
+
+def test__convert_shape_native_1d__tuple_input_is_returned_unchanged():
+    shape_native = (5,)
+    assert (
+        aa.util.geometry.convert_shape_native_1d(shape_native=shape_native)
+        is shape_native
+    )
+
+
+def test__convert_shape_native_1d__a_tracer_passes_through_untouched():
+    """Inside a `jax.jit` the value is traced; widening it would resolve it to a bool."""
+    tracer_like = _NotAConcreteScalar()
+
+    assert (
+        aa.util.geometry.convert_shape_native_1d(shape_native=tracer_like) is tracer_like
+    )
+
+
 def test__central_pixel_coordinates_1d_from():
     central_pixel_coordinates = aa.util.geometry.central_pixel_coordinates_1d_from(
         shape_slim=(3,)

test_autoarray/mask/test_mask_1d.py

@@ -55,6 +55,47 @@ def test__constructor__input_is_2d_mask__raises_exception():
         aa.Mask1D(mask=[[False, False, True]], pixel_scales=1.0)
 
 
+@pytest.mark.parametrize(
+    "pixel_scales", [1, 1.0, np.float64(1.0), np.int32(1), np.float32(1.0)]
+)
+def test__constructor__widens_any_real_scalar_pixel_scales(pixel_scales):
+    """
+    `Mask1D` hand-rolled its own `type(pixel_scales) is float` check and never routed through
+    `convert_pixel_scales_1d`, so an `int` or a NumPy scalar was stored bare. `Mask2D` already
+    went through the chokepoint — this closed the 1D/2D divergence.
+    """
+    mask = aa.Mask1D(mask=[False, False, True], pixel_scales=pixel_scales)
+
+    assert mask.pixel_scales == (1.0,)
+    assert type(mask.pixel_scales[0]) is float
+
+
+def test__constructor__scalar_pixel_scales__geometry_is_usable():
+    """
+    The bare scalar only surfaced later, when geometry subscripted it:
+    `TypeError: 'int' object is not subscriptable`, naming nothing the caller passed.
+    """
+    mask = aa.Mask1D(mask=[False, False, True], pixel_scales=1)
+
+    assert mask.geometry.scaled_maxima == (1.5,)
+
+
+def test__constructor__tuple_pixel_scales_returned_unchanged():
+    mask = aa.Mask1D(mask=[False, False, True], pixel_scales=(1.0,))
+
+    assert mask.pixel_scales == (1.0,)
+
+
+@pytest.mark.parametrize("pixel_scales", [0, 0.0, -1, -1.0, float("nan")])
+def test__constructor__invalid_pixel_scales__raises_exception(pixel_scales):
+    """
+    Routing through `convert_pixel_scales_1d` brings `validate.validate_pixel_scales` with it,
+    so `Mask1D` now rejects what `Mask2D` already rejected. A deliberate contract change.
+    """
+    with pytest.raises(ValueError):
+        aa.Mask1D(mask=[False, False, True], pixel_scales=pixel_scales)
+
+
 # ---------------------------------------------------------------------------
 # is_all_true / is_all_false — parametrized
 # ---------------------------------------------------------------------------