fix: reject non-Galaxy galaxies input to Tracer (#532 B4) - #696
Merged
Conversation
Tracer(galaxies="not a list") constructed happily and failed much later with
AttributeError: 'str' object has no attribute 'redshift' — an error naming
nothing the caller passed. It now raises TypeError at construction, naming
`galaxies` and the type that was given.
Broader than reported: 42, None and {'a': 1} were all accepted at construction
too, not only the reported string.
A string is the trap worth naming — it IS iterable, so an isinstance(x, Iterable)
check does not catch it. Elements are duck-typed on `redshift` rather than
required to be exactly ag.Galaxy, so mocks and Galaxy subclasses keep working;
`redshift` is precisely the attribute whose absence caused the original failure.
af.ModelInstance is accepted unexamined, since that is how PyAutoFit hands the
tracer a model's galaxies during a fit.
The negative-redshift half of #532 is NOT here: al.Galaxy IS ag.Galaxy, so both
the class and its redshift assignment live in PyAutoGalaxy, and the guard went
there (PyAutoGalaxy#566) where a bare Galaxy construction is also covered. A
Tracer-level redshift check would have missed that.
z_lens > z_source is phase 4 and stays HELD pending the reporter's answer. A
control test pins today's permissive behaviour so phase 4 cannot regress it
silently.
Tests: 13 new cases in test_autolens/lens/test_tracer_validation.py, built from
the reporter's snippet, with controls for list, tuple, empty list and
af.ModelInstance. Suite 519 passed, zero regressions.
Closes #532.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013PgqSCLTemK5bApVAwhVM4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #532. Phase 2 (PyAutoLens half) of the @rhayes777 API audit epic PyAutoArray#415, which stays open for phases 3-4.
What this fixes
The failure named nothing the caller passed, three frames from the constructor that accepted it. It now raises
TypeErrorat construction, naminggalaxiesand the type given.Broader than reported. The issue named the string case; verified on
mainthat42,Noneand{'a': 1}were all accepted at construction too.The string is the trap worth naming. A string is iterable, so an
isinstance(x, Iterable)check does not catch it — the element type is what matters, which the issue called out explicitly.Elements are duck-typed on
redshiftrather than required to be exactlyag.Galaxy, so mocks andGalaxysubclasses keep working — andredshiftis precisely the attribute whose absence produced the original error.af.ModelInstanceis accepted unexamined, since that is how PyAutoFit hands the tracer a model's galaxies during a fit.Where the redshift half went — and why not here
The negative-redshift finding on this issue is fixed in PyAutoGalaxy#566, not here.
It was filed on this issue because the reporter reached it through
al.Galaxy— butal.Galaxyisag.Galaxy, and both the class and itsredshiftassignment live in PyAutoGalaxy (autogalaxy/galaxy/galaxy.py:52). Guarding atTracerlevel would have missed a bareal.Galaxy(redshift=-0.5), which is exactly the reported reproduction. So the guard went where the attribute is set.Merging order does not matter — the two changes are independent.
Phase 4 stays held
z_lens > z_sourceis not implemented here. Multi-plane lensing genuinely supports geometries that look inverted under two-plane naming, so it must warn at most, never raise, and the question of whether even a warning is noise is still open with @rhayes777 on this issue.Rather than leave that implicit, this PR adds a guard-rail test pinning today's permissive behaviour: a
z_lens=1.0/z_source=0.5tracer must still construct and evaluate to a finite image. Phase 4 cannot quietly turn it into an error without that test failing. (PyAutoGalaxy#566 adds the matching guard-rail atGalaxylevel.)API Changes
No signatures, names or return types change. The change is in accepted input domain:
Tracer(galaxies=...)now requires alist/tupleof objects with aredshift, or anaf.ModelInstance. Strings, scalars,Noneand dicts were previously accepted and now raiseTypeError.An empty list remains valid — it is degenerate but legal, and used in tests and chaining.
Test Plan
test_autolens/lens/test_tracer_validation.py— 13 cases from the reporter's snippet, including the string trap, the wrong-container cases, an index-naming test for a bad list element, and controls for list, tuple, empty list andaf.ModelInstance.519 passed, 1 skipped. Zero regressions.Out of scope
z_lens > z_source— phase 4, HELD, with a guard-rail test added here.adapt_imagesprecondition — phase 3.PointSolver) — already closed in phase 1.Generated by Claude Code