Clarify typed test fixtures by marcpaterno · Pull Request #698 · LSSTDESC/firecrown

marcpaterno · 2026-10-25T01:06:28Z

Summary

Replace positional SACC and scale-cut fixture tuples with named, typed records. Update their consumers to access the intended fields directly, and use explicit np.float64 array types in the related Gaussian likelihood code.

tests/_types.py, tests/conftest.py     typed fixture records
tests/metadata/, tests/likelihood/    consumers and assertions
firecrown/likelihood/                 explicit array annotations

This PR builds on the strict test typing in #684.

Evidence

Before: Fixture meaning depended on tuple position, which was difficult for mypy and reviewers to verify.
After: make lint passes; the regular suite reports 4309 passed, 94 skipped, and 29 warnings.

Merge Danger

Door: Two-way. The fixture refactor can be reverted without a data migration.

Blast Radius: Tests. The main risk is an incorrectly updated fixture consumer; the full test suite passed.

github-actions · 2026-10-25T01:06:53Z

PR revision: 0632c88d68b2e00a155ef688e667477da2dfc5f7
Target: master

No breaking API changes detected.

PR public API verdict

No breaking API changes detected.

Target: master
Revision: 0632c88d68b2e00a155ef688e667477da2dfc5f7
Baseline: 15043d41af617b7f36abf2c8d630fd0325afaa66

Static analysis only: no behavioral compatibility guarantee or external-dependency signature analysis.

Public API changes: `15043d4` -> HEAD

Executive summary

No breaking API changes detected.

Breaking changes

None.

New public API

None.

Complete report artifact and workflow run | API job

codecov · 2026-10-25T01:28:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (1210152) to head (0632c88).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #698   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         163      163           
  Lines        9979     9979           
  Branches     1199     1199           
=======================================
  Hits         9979     9979

Files with missing lines	Coverage Δ
firecrown/likelihood/_gaussfamily.py	`100.0% <100.0%> (ø)`
firecrown/likelihood/_gaussian_pointmass.py	`100.0% <100.0%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vitenti · 2026-10-26T14:06:42Z

Hello @marcpaterno , a few things:

take a look at _gaussian_pointmass.py:451, it seems that this line is doing nothing.
Why do we need SaccHarmonicCrossBinFixture, SaccRealCrossBinFixture, SaccRealThreeTracerFixture and SaccHarmonicThreeTracerFixture. They seem to be equal, are we interested in differentiating by type?
The cross-bin and multi-bin SACC fixtures now return records, but the single-bin ones still return tuples: sacc_galaxy_cells_src0_src0, sacc_galaxy_cells_lens0_lens0, sacc_galaxy_xis_lens0_lens0, sacc_galaxy_cells_src0_src0_no_data, sacc_galaxy_cells_src0_src0_window and sacc_galaxy_cells_src0_src0_no_window return (sacc_data, TomographicBin), and sacc_galaxy_xis_lens0_lens0_real returns (sacc_data, z, thetas). Should they become records too, so all fixtures follow one convention?
Some docstrings could be shorter: several classes repeat what the types do not enforce ("Frozen bindings leave ... mutable", "do not check ... shapes, or units"), and "independent expectations" is misleading, since the expected arrays are the same ones passed to SACC.

Replace the tuple[object, ...] annotation on the sacc_galaxy_xis_lens0_lens0_real consumer with the actual (Sacc, float64, float64) contract matching its producer.

Replace bare np.ndarray tuple annotations with npt.NDArray[np.float64] in the two-point SACC metadata fixtures and their consuming tests. Convert the three-tracer fixture return annotations from parenthesized single-line forms to multi-line tuple forms. Add docstrings describing the fixture and test contracts, replacing the copied "without window functions" fixture descriptions.

Replace the tuple-based sacc_galaxy_cells fixture return with SaccGalaxyHarmonicFixture, which holds sacc_data, distributions, and spectra as frozen dataclasses (SampledRedshiftDistribution and HarmonicAngularPowerSpectrum). Update consumers to read named fields instead of positional indices. Add docstrings to the affected tests. Extend test_extract_all_data_harmonic_no_cov to run against a copy and assert that the shared fixture's covariance is left unchanged.

Replace the tuple-based `sacc_galaxy_xis` and `sacc_galaxy_xis_inverted` fixtures with `SaccGalaxyRealFixture`, which exposes `sacc_data`, `distributions`, and `spectra`. Each measured spectrum is a `RealAngularCorrelation` with `thetas` and `xis` fields instead of a positional pair. Tests now access fields by name and their docstrings describe the expected inputs. test_extract_all_data_real_no_cov now copies the shared SACC object before clearing its covariance. It also asserts that the shared fixture's covariance is unchanged, so the module-scoped fixture is not mutated for later tests.

Replace the tuple-based `SaccGalaxyWindowedFixture` and `WindowedAngularPowerSpectrum` aliases in tests/_types.py with frozen, keyword-only dataclasses. The fixture now exposes `sacc_data`, `distributions`, and `spectra` fields, and each spectrum exposes `support_ells`, `bandpowers`, and `window` instead of positional unpacking. Update conftest.py to build the new records, and update the metadata, types, and Gaussian likelihood tests to read the named fields. Test docstrings now describe the fixture contract. Behavior of the tests is unchanged.

Replace the tuple returned by the window_1 fixture with a frozen, keyword-only HarmonicWindowFunction dataclass holding ell_grid, weights, and band_labels. The two_point_cwindow and optimized_two_point_cwindow fixtures now read named fields instead of positional tuple indices, so the mapping onto TwoPointHarmonic's ells, window, and window_ells arguments is explicit. The dataclass docstring notes that its dtype annotations do not check weight-matrix shape or physical units.

Replace the positional tuple returned by the window function fixture with a frozen ScaleCutWindowFunction dataclass. Fields are accessed by name instead of index, so the weights, support multipoles, and bin centers can no longer be swapped silently. Test behavior is unchanged.

Introduce ScaleCutBandpowerPrediction and ScaleCutSaccFixture so the minimal_3x2pt_sacc fixture returns named fields instead of positional tuples. The label and support tests now read spectrum.bandpowers and spectrum.window instead of unpacking four-element tuples. The window fields reference the shared window_function_data directly rather than copying its arrays. Add explicit type annotations to the fixture locals and update the test docstrings to document their parameters.

Replace the tuple-returning cross-bin SACC fixtures with a SaccHarmonicCrossBinFixture dataclass. It holds the SACC data and a distributions mapping keyed by tracer name, so tests look up src0, src1, lens0, and lens1 by name instead of unpacking positional values. The fixtures in conftest.py and the tests that consume them in test_metadata_two_point.py and test_metadata_two_point_sacc.py are updated to match, and the affected tests now assert per-tracer z and dndz by name.

Replace tuple-returning real-space SACC fixtures with a typed SaccRealCrossBinFixture that exposes sacc_data and named per-tracer distributions. Update the consuming tests to read these fields and drop the now-unused numpy.typing imports.

Replace the positional tuples returned by the real-space and harmonic three-tracer fixtures with frozen keyword-only dataclasses. Each fixture now exposes `sacc_data` and a `distributions` mapping keyed by tracer name (src0, lens0, lens1), built from SampledRedshiftDistribution. Update the two three-tracer rejection tests to read `sacc_data` from the named fixture. Add explicit numpy array type annotations to the redshift and dN/dz arrays in both fixtures.

Annotate BACKGROUND with CCLCalculatorArgs and the PK_LINEAR and PK_NONLIN fragments with dict[str, PowerSpec], importing PowerSpec. The partial fragments keep the open mapping because no existing schema covers a single calculator-argument key.

Replace np.double annotations and builtin float dtypes with np.float64. These are equivalent aliases, so behavior is unchanged.

Replace tuple-returning SACC test fixtures with named frozen dataclasses (SaccGalaxyHarmonicFixture, SaccGalaxyRealFixture, SaccGalaxyWindowedFixture, SaccGalaxyDistributionFixture, SaccInvalidDataTypeFixture). Tests access sacc_data, distributions, and spectra by name instead of unpacking tuples. Relabel fixture expectations as synthetic observations rather than measured values, and define "Independent SACC expectation" and "Synthetic SACC observation" in GLOSSARY.md. Behavior of the existing tests is unchanged.

marcpaterno added this pull request to stack #699 October 25, 2026 01:06

marcpaterno changed the title ~~clarify test fixtures~~ Clarify typed test fixtures Oct 25, 2026

marcpaterno marked this pull request as ready for review October 25, 2026 03:16

marcpaterno force-pushed the clarify-test-fixtures branch from 7aa3895 to 39c30ed Compare October 27, 2026 20:24

Base automatically changed from improve-type-checking-in-tests to master October 28, 2026 02:13

marcpaterno added 16 commits October 27, 2026 21:13

refactor(test): type erased real-space fixture consumer annotation

de6c731

Replace the tuple[object, ...] annotation on the sacc_galaxy_xis_lens0_lens0_real consumer with the actual (Sacc, float64, float64) contract matching its producer.

refactor(test): document real-space TwoPoint fixture test

6d55bf5

refactor(test): type sync deps requirement cases with dataclass

beea667

refactor(likelihood): use explicit np.float64 dtypes

7df37df

Replace np.double annotations and builtin float dtypes with np.float64. These are equivalent aliases, so behavior is unchanged.

marcpaterno force-pushed the clarify-test-fixtures branch from 2fe237f to 0632c88 Compare October 28, 2026 02:13

Navigation Menu

Repository navigation

Clarify typed test fixtures - #698

Clarify typed test fixtures#698
marcpaterno wants to merge 16 commits into
masterfrom
clarify-test-fixtures

marcpaterno commented Oct 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 25, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 25, 2026 •

edited

Loading

Uh oh!

vitenti commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

marcpaterno commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Evidence

Merge Danger

Uh oh!

github-actions Bot commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR public API verdict

Public API changes: 15043d4 -> HEAD

Executive summary

Breaking changes

New public API

Uh oh!

codecov Bot commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vitenti commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marcpaterno commented Oct 25, 2026 •

edited

Loading

github-actions Bot commented Oct 25, 2026 •

edited

Loading

Public API changes: `15043d4` -> HEAD

codecov Bot commented Oct 25, 2026 •

edited

Loading