From 8298d74e1b9f7607d973300cf9631e98d6d290c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 01:49:04 +0000 Subject: [PATCH] fix: widen int / numpy scalar pixel_scales to a tuple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `convert_pixel_scales_1d` and `convert_pixel_scales_2d` tested `type(pixel_scales) is float`, an exact-type check, so only a literal Python `float` was widened to the tuple form both functions promise. An `int`, an `np.floating` or an `np.integer` fell through unconverted. The two functions are the single chokepoint that every `Mask2D` factory, `Grid2D.uniform` and the `Array2D`/`Array1D`/`Grid1D` constructors funnel `pixel_scales` through — 16 call sites — so the broken promise was repeated across the public API. `Array2D.no_mask(values=..., pixel_scales=1)` stored the bare `1` on the mask and every later use of it raised `TypeError: 'int' object is not subscriptable`, naming nothing the caller passed; `Grid2D.uniform` and `Mask2D.circular` raised it outright. An `int` is a natural thing to type by hand, and an `np.floating` is what indexing an array or reading a FITS header returns, so both arrive on ordinary paths. Test any concrete real scalar instead, via `validate.is_concrete_scalar`, and cast the widened value to a Python `float` so the returned tuple matches the `Tuple[float, ...]` annotation regardless of input type — this also keeps NumPy scalars out of stored geometry and out of JSON serialisation. `is_concrete_scalar` excludes `bool` and JAX tracers, so a traced value still passes through untouched and the function stays safe inside a `jax.jit`. The validation guards run ahead of the widening and are unchanged, so `0`, `-1` and `nan` are still rejected — now in their integer and NumPy forms too. Applied to the 1D and 2D siblings together so they cannot drift apart. Pre-existing defect, not a regression; the follow-up #440 pointed at. Not fixed here, each needing its own change: tuple entries are still returned unnormalised (`(1, 1)` stays ints), and `convert_shape_native_1d` carries the identical `type(x) is int` exact-type check for `np.integer`. Validation: 1145 passed / 1 skipped / 3 failed; the 3 are the known pre-existing pynufft failures in `test_transformer.py`, baselined by re-running them on a clean tree (identical 3). 27 new tests, all confirmed to fail without the source change. Closes #464 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013th2YEQwwnoiA7aEufSutT --- autoarray/geometry/geometry_util.py | 52 ++++++++---- autoarray/type.py | 7 +- test_autoarray/geometry/test_geometry_util.py | 83 +++++++++++++++++++ test_autoarray/test_validate.py | 51 ++++++++++++ 4 files changed, 177 insertions(+), 16 deletions(-) diff --git a/autoarray/geometry/geometry_util.py b/autoarray/geometry/geometry_util.py index f3a72ed4c..d3e83cc37 100644 --- a/autoarray/geometry/geometry_util.py +++ b/autoarray/geometry/geometry_util.py @@ -32,16 +32,27 @@ def convert_shape_native_1d(shape_native: Union[int, Tuple[int]]) -> Tuple[int]: def convert_pixel_scales_1d(pixel_scales: ty.PixelScales) -> Tuple[float]: """ - Convert an input pixel scale of type `float` to a tuple `(float,)`. If the input is already a - `(float,)` tuple it is returned unchanged. + Convert an input pixel scale given as a single real scalar to a tuple `(float,)`. If the + input is already a `(float,)` tuple it is returned unchanged. - This enables users to input the pixel scale as a single float and have the type automatically - normalised to `(float,)` which is used internally by 1D data structures. + This enables users to input the pixel scale as a single number and have the type + automatically normalised to `(float,)` which is used internally by 1D data structures. + + Any concrete real scalar is widened — `int`, `float`, `np.integer` and `np.floating` — not + just an exact `float`. An `int` is a natural thing to type by hand, and an `np.floating` is + what indexing a numpy array or reading a FITS header returns, so both reach this function on + paths a user would consider ordinary. The widened value is cast to a Python `float`, so the + tuple this returns is always `(float,)` regardless of what went in. + + A `bool` is deliberately *not* treated as a scalar here (see + :func:`autoarray.validate.is_concrete_scalar`), and neither is a JAX tracer — a traced value + passes through untouched so the function stays safe inside a `jax.jit`. Parameters ---------- pixel_scales - The pixel scale to convert, either as a plain `float` or a 1-element tuple `(float,)`. + The pixel scale to convert, either as a plain real scalar or a 1-element tuple + `(float,)`. Returns ------- @@ -56,8 +67,8 @@ def convert_pixel_scales_1d(pixel_scales: ty.PixelScales) -> Tuple[float]: validate.validate_pixel_scales(pixel_scales=pixel_scales) - if type(pixel_scales) is float: - pixel_scales = (pixel_scales,) + if validate.is_concrete_scalar(pixel_scales): + pixel_scales = (float(pixel_scales),) return pixel_scales @@ -197,17 +208,28 @@ def scaled_coordinates_1d_from( def convert_pixel_scales_2d(pixel_scales: ty.PixelScales) -> Tuple[float, float]: """ - Convert an input pixel scale of type `float` to a tuple `(float, float)`. If the input is - already type `(float, float)` it is returned unchanged. + Convert an input pixel scale given as a single real scalar to a tuple `(float, float)`. If + the input is already type `(float, float)` it is returned unchanged. + + This enables users to input the pixel scale as a single number and have the type + automatically normalised to `(float, float)` which is used internally for rectangular 2D + grids (where both axes share the same pixel scale). + + Any concrete real scalar is widened — `int`, `float`, `np.integer` and `np.floating` — not + just an exact `float`. An `int` is a natural thing to type by hand, and an `np.floating` is + what indexing a numpy array or reading a FITS header returns, so both reach this function on + paths a user would consider ordinary. The widened value is cast to a Python `float`, so the + tuple this returns is always `(float, float)` regardless of what went in. - This enables users to input the pixel scale as a single float and have the type automatically - normalised to `(float, float)` which is used internally for rectangular 2D grids (where - both axes share the same pixel scale). + A `bool` is deliberately *not* treated as a scalar here (see + :func:`autoarray.validate.is_concrete_scalar`), and neither is a JAX tracer — a traced value + passes through untouched so the function stays safe inside a `jax.jit`. Parameters ---------- pixel_scales - The pixel scale to convert, either as a plain `float` or a 2-element tuple `(float, float)`. + The pixel scale to convert, either as a plain real scalar or a 2-element tuple + `(float, float)`. Returns ------- @@ -228,8 +250,8 @@ def convert_pixel_scales_2d(pixel_scales: ty.PixelScales) -> Tuple[float, float] validate.validate_pixel_scales(pixel_scales=pixel_scales) - if type(pixel_scales) is float: - pixel_scales = (pixel_scales, pixel_scales) + if validate.is_concrete_scalar(pixel_scales): + pixel_scales = (float(pixel_scales), float(pixel_scales)) return pixel_scales diff --git a/autoarray/type.py b/autoarray/type.py index f84ee91aa..ea6b4e31f 100644 --- a/autoarray/type.py +++ b/autoarray/type.py @@ -1,7 +1,12 @@ import numpy as np from typing import List, Tuple, Union -PixelScales = Union[Tuple[float], Tuple[float, float], float] +# A pixel scale may be given per-axis as a tuple, or as a single real scalar which +# `geometry_util.convert_pixel_scales_{1d,2d}` widens to that tuple. The scalar forms are +# listed out because the widening accepts any real scalar, not just an exact `float`. +PixelScales = Union[ + Tuple[float], Tuple[float, float], float, int, np.floating, np.integer +] from autoarray.mask.mask_1d import Mask1D diff --git a/test_autoarray/geometry/test_geometry_util.py b/test_autoarray/geometry/test_geometry_util.py index 0bd85a267..a487bd005 100644 --- a/test_autoarray/geometry/test_geometry_util.py +++ b/test_autoarray/geometry/test_geometry_util.py @@ -3,6 +3,89 @@ import pytest +class _NotAConcreteScalar: + """ + Stand-in for a JAX tracer: not a concrete Python/NumPy scalar, and raising if anything + tries to resolve it to a bool, exactly as a tracer does inside `jax.jit`. Mirrors the + stand-in in `test_autoarray/test_validate.py` — unit tests here are NumPy-only. + """ + + def __bool__(self): + raise AssertionError("a tracer must never be resolved to a bool") + + +@pytest.mark.parametrize( + "pixel_scales", [1, 1.0, np.float64(1.0), np.int32(1), np.float32(1.0)] +) +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. + """ + assert aa.util.geometry.convert_pixel_scales_1d(pixel_scales=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, + ) + + +@pytest.mark.parametrize("pixel_scales", [1, np.float64(1.0), np.int32(1)]) +def test__convert_pixel_scales__widened_entries_are_python_floats(pixel_scales): + """ + The widened value is cast, so an `int` or a NumPy scalar never reaches the geometry + stored on a mask. `1 == 1.0` in Python, so the cast has to be asserted on the type. + """ + (entry_1d,) = aa.util.geometry.convert_pixel_scales_1d(pixel_scales=pixel_scales) + assert type(entry_1d) is float + + for entry in aa.util.geometry.convert_pixel_scales_2d(pixel_scales=pixel_scales): + assert type(entry) is float + + +def test__convert_pixel_scales__tuple_input_is_returned_unchanged(): + pixel_scales_1d = (1.0,) + assert ( + aa.util.geometry.convert_pixel_scales_1d(pixel_scales=pixel_scales_1d) + is pixel_scales_1d + ) + + pixel_scales_2d = (1.0, 2.0) + assert ( + aa.util.geometry.convert_pixel_scales_2d(pixel_scales=pixel_scales_2d) + is pixel_scales_2d + ) + + +def test__convert_pixel_scales__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_pixel_scales_1d(pixel_scales=tracer_like) + is tracer_like + ) + assert ( + aa.util.geometry.convert_pixel_scales_2d(pixel_scales=tracer_like) + is tracer_like + ) + + +def test__convert_pixel_scales__a_bool_is_not_treated_as_a_scalar(): + """ + `bool` is a subclass of `int`, but `True` reaching a pixel scale is a different mistake + than the ones this widening serves — `validate.is_concrete_scalar` excludes it, so it is + not silently accepted as a pixel scale of 1.0. + """ + assert aa.util.geometry.convert_pixel_scales_1d(pixel_scales=True) is True + assert aa.util.geometry.convert_pixel_scales_2d(pixel_scales=True) is True + + 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/test_validate.py b/test_autoarray/test_validate.py index 1277e2b22..3dc548b9a 100644 --- a/test_autoarray/test_validate.py +++ b/test_autoarray/test_validate.py @@ -124,6 +124,57 @@ def test__b6__control__valid_pixel_scales_still_build(): assert array.pixel_scales == (0.1, 0.2) +@pytest.mark.parametrize("pixel_scales", [0, -1, np.int32(0), np.int32(-1)]) +def test__b6__the_guards_still_fire_on_integer_scalars(pixel_scales): + """ + The guards run before the widening, so broadening what counts as a scalar must not open a + hole for the integer forms of the same bad input. + """ + with pytest.raises(ValueError, match="pixel_scales"): + aa.Array2D.no_mask(values=np.ones((5, 5)), pixel_scales=pixel_scales) + + +@pytest.mark.parametrize( + "pixel_scales", [np.float64(0.0), np.float64(-0.1), np.float64("nan")] +) +def test__b6__the_guards_still_fire_on_numpy_scalars(pixel_scales): + with pytest.raises(ValueError, match="pixel_scales"): + aa.Array2D.no_mask(values=np.ones((5, 5)), pixel_scales=pixel_scales) + + +@pytest.mark.parametrize("pixel_scales", [(0, 1), (1, -1)]) +def test__b6__the_guards_still_fire_on_integer_entries_of_a_tuple(pixel_scales): + with pytest.raises(ValueError, match="pixel_scales"): + aa.Array2D.no_mask(values=np.ones((5, 5)), pixel_scales=pixel_scales) + + +@pytest.mark.parametrize("pixel_scales", [1, np.float64(1.0), np.int32(1)]) +def test__b6__control__an_integer_or_numpy_pixel_scale_builds_and_is_widened( + pixel_scales, +): + """ + The defect this closes: only an exact `float` used to be widened, so an `int` or a NumPy + scalar was stored on the mask unconverted and every later use of it raised + `TypeError: 'int' object is not subscriptable`. + """ + array = aa.Array2D.no_mask(values=np.ones((5, 5)), pixel_scales=pixel_scales) + assert array.pixel_scales == (1.0, 1.0) + assert array.pixel_scale == 1.0 + + mask = aa.Mask2D.circular( + shape_native=(5, 5), radius=2.0, pixel_scales=pixel_scales + ) + assert mask.pixel_scales == (1.0, 1.0) + + grid = aa.Grid2D.uniform(shape_native=(5, 5), pixel_scales=pixel_scales) + assert grid.pixel_scales == (1.0, 1.0) + + +def test__b6__control__an_integer_pixel_scale_builds_a_1d_structure(): + array = aa.Array1D.no_mask(values=np.ones((5,)), pixel_scales=1) + assert array.pixel_scales == (1.0,) + + # ====================================================================================== # B7 — annulus radii must be ordered # ======================================================================================