Add hilbert_pixels_from_pixel_scale to model_util#288
Conversation
Returns recommended Hilbert-curve pixel counts based on image pixel scale, with unit tests covering all branches and boundary values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a small helper in autogalaxy.analysis.model_util to map an image pixel scale to a recommended Hilbert-curve pixel count, and expands the existing model_util test suite to cover the new mapping logic.
Changes:
- Add
hilbert_pixels_from_pixel_scale(pixel_scale: float) -> intwith hard-coded threshold mapping. - Add unit tests covering the new function’s branches and selected boundary values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
autogalaxy/analysis/model_util.py |
Introduces hilbert_pixels_from_pixel_scale to recommend Hilbert pixel counts from pixel scale. |
test_autogalaxy/analysis/test_model_util.py |
Adds tests for the new helper across thresholds and boundary cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if pixel_scale > 0.06: | ||
| return 1000 | ||
| elif pixel_scale > 0.04: | ||
| return 1250 | ||
| elif pixel_scale >= 0.03: | ||
| return 1500 | ||
| else: | ||
| return 1750 |
There was a problem hiding this comment.
hilbert_pixels_from_pixel_scale silently returns a value for non-physical inputs (e.g. pixel_scale <= 0 or NaN), which can mask upstream bugs and produce misleading pixel counts. Consider validating that pixel_scale is finite and > 0 and raising a ValueError (similar to the validation in mge_point_model_from).
|
|
||
|
|
||
| def test__hilbert_pixels_from_pixel_scale__between_004_and_006(): | ||
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.05) == 1250 | ||
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.061) == 1000 |
There was a problem hiding this comment.
This test name says it covers pixel scales between 0.04 and 0.06, but it also asserts a value for 0.061 (which is > 0.06 and therefore belongs to the "above 0.06" branch). Renaming the test or moving the 0.061 assertion to the appropriate test would make the test intent match the behavior being exercised.
| def test__hilbert_pixels_from_pixel_scale__between_004_and_006(): | |
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.05) == 1250 | |
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.061) == 1000 | |
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.061) == 1000 | |
| def test__hilbert_pixels_from_pixel_scale__between_004_and_006(): | |
| assert ag.model_util.hilbert_pixels_from_pixel_scale(0.05) == 1250 |
Raise ValueError for non-positive or non-finite inputs to catch upstream bugs early, consistent with the validation in mge_point_model_from. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Returns recommended Hilbert-curve pixel counts based on image pixel scale, with unit tests covering all branches and boundary values.