Skip to content

feat: Shape.contains / Shape.boundary for source-plane shapes (image-source-mappings P2a) - #518

Merged
Jammy2211 merged 3 commits into
mainfrom
feature/image-source-mappings-p2
Sep 2, 2026
Merged

Jammy2211 merged 3 commits into
mainfrom
feature/image-source-mappings-p2

Conversation

@Jammy2211

@Jammy2211 Jammy2211 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Phase 2a of the image-source-mappings epic: point containment and boundary polygons for the source-plane Shape classes, plus three defects the Phase 2 ShapeSolver validation suite exposed in PyAutoArray's triangle and overlay code. This is the PyAutoArray half of PyAutoLabs/PyAutoLens#719; the PyAutoLens PR (PyAutoLabs/PyAutoLens#720) depends on it — its CI is source-installed against PyAutoArray main, so it goes green once this merges. Merge order: this PR first.

  • Shape.contains(points) -> bool array on every shape: Circle radial, Triangle barycentric, Polygon any-of its fan triangles (exact for convex polygons), Square bounds; Point.contains raises, pointing at Circle / PointSolver.
  • Shape.boundary(n=100) -> (n, 2) closed loop for drawing the source-plane region.
  • The coordinate convention is documented once on the Shape base class: points use the same axis order as triangles[..., 0] / [..., 1] that mask() already reads, and PointSolver.solve builds Point(*source_plane_coordinate) from a (y, x) tuple, so the attribute named x holds the first (y) coordinate.

Audit findings (all fixed here, each with a regression test)

Finding Commit Evidence
Triangle.triangle_contains_mask tested the reflected triangle: it unpacked vertices as y1, x1 = self.a (element 1 as "x") while testing a centroid built as (element 0, element 1). On the asymmetric triangle ((0,0),(3,0),(0,1)) the interior point (2.0, 0.2) read as outside and its reflection (0.2, 2.0) as inside. Every symmetric shape in the existing test_polygons.py hid it. All three barycentric copies now share one _barycentric_contains helper. b1f88f4a test_shape.py::test_triangle_contains_mask_is_not_reflected; ::test_contains_uses_the_same_axis_order_as_the_triangle_array pins the convention empirically
CoordinateArrayTriangles{,Np}.for_limits_and_scale named its first pair of limits x_min/x_max and tiled them along element 0, while element 0 is y everywhere else (ArrayTrianglesNp, every (y, x) grid, the Shape convention). Both the keyword caller (AbstractSolver._initial_triangles) and the positional caller (AbstractTriangles.for_grid) tiled the transposed rectangle. Invisible on a square grid; on a 24×80 grid of 0.05" pixels PointSolver.solve found one of an Isothermal's two images instead of both, the missing one well inside the grid. 67b1cb75 test_coordinate.py::test_for_limits_and_scale__element_0_spans_the_y_limits, ::test_for_grid__rectangular_grid_is_tiled_the_same_way_round
Stray bbbb statement in ArrayTrianglesNp.with_vertices — a NameError on every call since 2025-11, unreached because the solver only uses the CoordinateArrayTrianglesNp override. 67b1cb75 test_coordinate.py::test_array_triangles_with_vertices_returns_new_triangles
plot_regions drew one label per region entry at the mean of all its polygons, so a mapping's label for two multiple images landed on empty sky between them. Multi-polygon regions now carry the label once per polygon. a9a9120a new tests in test_autoarray/plot/ (a two-polygon region with one label yields two text artists)

Square.mask / Square.area assume top < bottom numerically, consistent with their "coordinates from the top-left corner" docstring — not a bug. The new contains / boundary sort the bounds so they also work for arcsec (y, x) squares (test_shape.py::test_square_contains_is_robust_to_coordinate_ordering).

API Changes

  • Added Shape.contains(points) and Shape.boundary(n=100) (abstract on Shape; implemented on Point, Circle, Triangle, Polygon, Square).
  • Changed CoordinateArrayTriangles.for_limits_and_scale / CoordinateArrayTrianglesNp.for_limits_and_scale signature order from (x_min, x_max, y_min, y_max, scale) to (y_min, y_max, x_min, x_max, scale), matching the abstract base and the positional caller. Keyword callers are unaffected by the reorder and now get the rectangle they asked for. Behaviour change: PointSolver on a non-square grid now searches the grid's actual extent and can return multiple images it previously missed.
  • Changed plot_regions label placement for multi-polygon regions (one label per polygon).
  • mask(triangles), area, the pytree methods and autoarray/__init__.py are untouched.

Tests

  • New test_autoarray/structures/triangles/test_shape.py (19 tests) and additions to test_coordinate.py (3) and test_autoarray/plot/ (2).
  • pytest test_autoarray -q: 1410 passed (Phase 1 shipped at 1382).

Epic

image-source-mappings phase 2a — ledger PyAutoMind/draft/feature/autoarray/image_source_mappings_epic.md. Phase 1 was PyAutoArray#517. Label pending-release.

🤖 Generated with Claude Code

https://claude.ai/code/session_011EVnQ4sEYrRM7GCvFyveUA

…e-source-mappings phase 2a)

Point containment and closed boundary polygons for Circle, Triangle, Polygon and
Square, documenting the (y, x) axis convention the triangle solvers already use.
Fixes the reflected containment test in Triangle.triangle_contains_mask, which
unpacked its vertices as (y, x) while testing a centroid built as (x, y).
Square.contains / Square.boundary sort their bounds so they work for either
top/bottom ordering; mask and area keep their documented assumption.

Part of PyAutoLabs/PyAutoLens#719 (image-source-mappings epic, phase 2a).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EVnQ4sEYrRM7GCvFyveUA
Jammy2211 and others added 2 commits September 2, 2026 18:08
…e-source-mappings phase 2a)

`CoordinateArrayTriangles{,Np}.for_limits_and_scale` named its first pair of limits
`x_min`/`x_max` and tiled them along element 0 of every vertex, while element 0 is the
`y` coordinate everywhere else — `ArrayTrianglesNp.for_limits_and_scale`, every PyAuto
`(y, x)` grid, and the `element 0 <-> element 0` convention `Shape.contains` / `Shape.mask`
are documented with. Both the keyword caller (`AbstractSolver._initial_triangles`) and the
positional caller (`AbstractTriangles.for_grid`) therefore tiled the transposed rectangle.
A square grid hides this, which is why it shipped; on a 24x80 grid of 0.05" pixels
`PointSolver.solve` found one of an Isothermal's two images instead of both, the missing
one lying well inside the grid.

Also removes a stray `bbbb` statement (a `NameError` on every call) left in
`ArrayTrianglesNp.with_vertices` since 2025-11-13, uncaught because the solver only ever
reaches the `CoordinateArrayTrianglesNp` override of that name.

Found by the ShapeSolver validation suite of image-source-mappings phase 2.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EVnQ4sEYrRM7GCvFyveUA
…mappings phase 2a)

`plot_regions` drew one label per region entry, at the mean of all of that region's
polygons. A region whose polygons are the multiple images of one lensed source has them
on opposite sides of the lens, so the label landed at their midpoint -- on empty sky,
labelling nothing, which is exactly the case the mappings overlay exists for. The label
is now repeated once per polygon, at that polygon's own centre. A single-polygon region
is unchanged: its polygon's mean is the region's mean.

Found by the phase 2 verification renders.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EVnQ4sEYrRM7GCvFyveUA
@Jammy2211
Jammy2211 merged commit c9f67e7 into main Sep 2, 2026
3 checks passed
@Jammy2211
Jammy2211 deleted the feature/image-source-mappings-p2 branch September 2, 2026 22:55
@Jammy2211 Jammy2211 removed the pending-release PR queued for the next release build label Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant