diff --git a/autoarray/geometry/geometry_util.py b/autoarray/geometry/geometry_util.py index d3e83cc37..5f63cbf6d 100644 --- a/autoarray/geometry/geometry_util.py +++ b/autoarray/geometry/geometry_util.py @@ -7,16 +7,25 @@ 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 ------- @@ -24,8 +33,8 @@ def convert_shape_native_1d(shape_native: Union[int, Tuple[int]]) -> 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 diff --git a/autoarray/mask/mask_1d.py b/autoarray/mask/mask_1d.py index 24e3e226e..511266e2a 100644 --- a/autoarray/mask/mask_1d.py +++ b/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") diff --git a/autoarray/validate.py b/autoarray/validate.py index b7e40802a..f95c9e979 100644 --- a/autoarray/validate.py +++ b/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, diff --git a/test_autoarray/geometry/test_geometry_util.py b/test_autoarray/geometry/test_geometry_util.py index a487bd005..bf5b64f2d 100644 --- a/test_autoarray/geometry/test_geometry_util.py +++ b/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,) diff --git a/test_autoarray/mask/test_mask_1d.py b/test_autoarray/mask/test_mask_1d.py index bf8a0a535..7f466d5b6 100644 --- a/test_autoarray/mask/test_mask_1d.py +++ b/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 # ---------------------------------------------------------------------------