Skip to content

Refactor unit tests: granularity, descriptive names, parametrize#405

Merged
Jammy2211 merged 4 commits intomainfrom
claude/refactor-unit-tests
Mar 25, 2026
Merged

Refactor unit tests: granularity, descriptive names, parametrize#405
Jammy2211 merged 4 commits intomainfrom
claude/refactor-unit-tests

Conversation

@Jammy2211
Copy link
Copy Markdown
Collaborator

Summary

  • Split omnibus tests into focused single-scenario tests in test_fit_imaging.py, test_tracer.py, and all point-source fit files — each test now covers exactly one scenario so a failure immediately identifies the broken case
  • Descriptive test names now encode the input condition and expected outcome (e.g. test__figure_of_merit__pixelization_inversion__expected_log_evidence instead of test__fit_figure_of_merit)
  • pytest.mark.parametrize applied to multi-class/multi-case tests: FitPointDataset position fitting classes, plane_index_via_redshift_from, extract_plane_index_of_profile, upper plane index with light profile
  • New plot tests covering functions added in the recent rewrite of fit_imaging_plots.py and tracer_plots.py that had zero coverage: subplot_fit_x1_plane, subplot_fit_log10_x1_plane, subplot_tracer_from_fit, subplot_fit_combined, subplot_fit_combined_log10, save_tracer_fits, save_source_plane_images_fits
  • All numerical assertions preserved exactly unchanged

Files changed

File Change
test_autolens/imaging/test_fit_imaging.py Split test__fit_figure_of_merit (10+ scenarios) + sub_2 variants into individual tests; split dict and subtracted image tests
test_autolens/lens/test_tracer.py Split test__has, plane/galaxy sorting tests, convergence/potential/deflection tests; parametrize redshift lookups and extract_plane_index
test_autolens/imaging/plot/test_fit_imaging_plots.py Add 7 new tests for uncovered plot functions; split test__subplot_of_planes
test_autolens/lens/plot/test_tracer_plots.py Add 2 new tests for save_tracer_fits / save_source_plane_images_fits using tmp_path
test_autolens/point/fit/test_fit_dataset.py Split by matching/non-matching name; parametrize position fitting classes
test_autolens/point/fit/test_fluxes.py Rename tests to be self-describing
test_autolens/point/fit/positions/image/test_pair*.py Rename + split multi-scenario tests
test_autolens/point/fit/positions/image/test_abstract.py Rename for clarity
test_autolens/point/fit/positions/source/test_separations.py Split 2 inline scenarios into 3 focused tests

Test plan

  • python -m pytest test_autolens/imaging/test_fit_imaging.py -v
  • python -m pytest test_autolens/lens/test_tracer.py -v
  • python -m pytest test_autolens/imaging/plot/ -v
  • python -m pytest test_autolens/lens/plot/ -v
  • python -m pytest test_autolens/point/fit/ -v

🤖 Generated with Claude Code

Jammy2211 and others added 2 commits March 25, 2026 19:53
- Split omnibus tests into focused single-scenario tests throughout
  test_fit_imaging.py, test_tracer.py, and point fit files so failures
  pinpoint the exact broken scenario rather than a broad test function
- Renamed all tests to encode the input condition and expected outcome
  (e.g. test__figure_of_merit__pixelization_inversion__expected_log_evidence)
- Parametrized multi-class/multi-redshift cases with pytest.mark.parametrize:
  FitPointDataset position fitting classes, plane_index_via_redshift_from,
  extract_plane_index_of_profile, and upper_plane_index_with_light_profile
- Added coverage for new plotting functions lacking any tests:
  subplot_fit_x1_plane, subplot_fit_log10_x1_plane, subplot_tracer_from_fit,
  subplot_fit_combined, subplot_fit_combined_log10, save_tracer_fits,
  save_source_plane_images_fits
- All numerical assertions preserved exactly unchanged

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consistent with test_fluxes.py rename pattern: test names now encode
the tracer type used (mock vs real isothermal).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the AutoLens test suite to make failures more actionable by splitting multi-scenario “omnibus” tests into single-purpose tests, improving test naming, and using pytest.mark.parametrize where appropriate. It also adds new plot and FITS-output tests to cover recently introduced plotting / output helpers that previously had no coverage.

Changes:

  • Split large multi-scenario tests into focused single-scenario tests and renamed tests to encode conditions + expected outcomes.
  • Added parametrized tests for multi-class / multi-case behavior (e.g., point position fitting classes, redshift/plane-index helpers).
  • Added new plot and FITS-writing tests for tracer / fit imaging plotting utilities.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test_autolens/point/fit/test_time_delays.py Renames / clarifies time-delay fit tests (mock tracer vs real tracer).
test_autolens/point/fit/test_fluxes.py Renames / clarifies flux fit tests (mock tracer vs real tracer).
test_autolens/point/fit/test_fit_dataset.py Splits matching vs non-matching point name tests; parametrizes position-fit class expectations; factors common fixtures.
test_autolens/point/fit/positions/source/test_separations.py Splits combined scenarios into separate tests for mass/no-mass and multi-plane behavior.
test_autolens/point/fit/positions/image/test_pair_repeat.py Splits repeat-allocation behavior into separate focused tests with full quantity assertions.
test_autolens/point/fit/positions/image/test_pair_all.py Renames tests to be scenario-specific and keeps edge-case coverage (inf / duplicates).
test_autolens/point/fit/positions/image/test_pair.py Renames the image-pair residuals test to be explicit about behavior.
test_autolens/point/fit/positions/image/test_abstract.py Refocuses on multi-plane scaling behavior for the abstract fit class.
test_autolens/lens/test_tracer.py Splits and parametrizes tracer behavior tests for clearer single-failure diagnosis.
test_autolens/lens/plot/test_tracer_plots.py Adds tests for FITS-writing helpers (save_tracer_fits, save_source_plane_images_fits) using tmp_path.
test_autolens/imaging/test_fit_imaging.py Major split of FitImaging tests into granular, scenario-specific tests while preserving numeric assertions.
test_autolens/imaging/plot/test_fit_imaging_plots.py Adds coverage for new plotting helpers and splits plane-subplot tests for clearer intent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +3
import numpy as np
import pytest

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

numpy and pytest are imported but not used in this test file. Consider removing them to keep the test minimal and reduce noise.

Suggested change
import numpy as np
import pytest

Copilot uses AI. Check for mistakes.
# Replace 2.5 with expected model time delay from your tracer
assert fit.model_time_delays.in_list[1] == pytest.approx(-573.994580905, 1.0e-4)
assert fit.log_likelihood == pytest.approx(-22600.81488747, 1.0e-4)
import numpy as np
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

numpy is imported but not used in this test file. Consider removing the unused import to keep the test focused.

Suggested change
import numpy as np

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +45
assert fit_0.model_data[0, :] == pytest.approx(
scaling_factor * fit_1.model_data.array[0, :], 1.0e-1
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two assertions are identical, so the second one is redundant and may be a copy/paste mistake (it looks like you intended to assert a different row / index). Consider removing the duplicate or changing the second assertion to cover the other coordinate/component you meant to validate.

Suggested change
assert fit_0.model_data[0, :] == pytest.approx(
scaling_factor * fit_1.model_data.array[0, :], 1.0e-1
assert fit_0.model_data[1, :] == pytest.approx(
scaling_factor * fit_1.model_data.array[1, :], 1.0e-1

Copilot uses AI. Check for mistakes.
assert fit_0.model_data[0, :] == pytest.approx(
scaling_factor * fit_1.model_data.array[0, :], 1.0e-1
)
import numpy as np
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

numpy is imported but no longer used in this test file. Removing unused imports keeps the tests easier to maintain and avoids confusion about expected dependencies.

Suggested change
import numpy as np

Copilot uses AI. Check for mistakes.
Comment on lines +552 to +561
masked_imaging_covariance_7x7, image_7x7, psf_3x3, noise_map_7x7, mask_2d_7x7
):
dataset = al.Imaging(
data=image_7x7,
psf=psf_3x3,
noise_map=noise_map_7x7,
over_sample_size_lp=2,
)
masked_imaging_7x7 = dataset.apply_mask(mask=mask_2d_7x7)

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test builds a new Imaging dataset and masked_imaging_7x7, but then fits using masked_imaging_covariance_7x7 instead, so the dataset created here is unused. Either remove this unused setup, or use the newly created masked dataset so the test actually exercises the intended sub-2 oversampling path.

Suggested change
masked_imaging_covariance_7x7, image_7x7, psf_3x3, noise_map_7x7, mask_2d_7x7
):
dataset = al.Imaging(
data=image_7x7,
psf=psf_3x3,
noise_map=noise_map_7x7,
over_sample_size_lp=2,
)
masked_imaging_7x7 = dataset.apply_mask(mask=mask_2d_7x7)
masked_imaging_covariance_7x7,
):

Copilot uses AI. Check for mistakes.
Jammy2211 and others added 2 commits March 25, 2026 20:12
…rtion

- test_abstract.py: remove unused `import numpy as np`; fix duplicate
  assertion so second check covers row [1, :] not [0, :] again
- test_fit_imaging.py: remove unused fixtures and dead setup block from
  covariance test (only masked_imaging_covariance_7x7 is needed)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…index

The second assert was a copy-paste of the first (both checked row [0, :]).
Changing it to row [1, :] introduced a genuinely failing assertion since
the scaling relationship does not hold for the second position. Simply
remove the duplicate instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Jammy2211 Jammy2211 merged commit e1d4678 into main Mar 25, 2026
8 checks passed
@Jammy2211 Jammy2211 deleted the claude/refactor-unit-tests branch April 2, 2026 11:44
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.

2 participants