From d98bec3cecc737b24a0155e805855d1cd7fa391e Mon Sep 17 00:00:00 2001 From: Kieran Leschinski Date: Wed, 19 Aug 2026 15:38:57 +0200 Subject: [PATCH 1/3] Pass the opened HDUList through rectify_traces and rectify_cube Both methods accept a filename or an HDUList and open the file into inhdul, but then handed the original argument to make_image_interpolations and SpectralTrace.rectify. With a filename this crashed (a str cannot be indexed as an HDUList); with an in-memory HDUList it happened to work, masking the bug. Use inhdul consistently after opening. Adds a regression test that feeds a filename through rectify_traces and asserts the traces receive the opened HDUList. --- scopesim/effects/metis_lms_trace_list.py | 2 +- scopesim/effects/spectral_trace_list.py | 4 +- .../tests_effects/test_SpectralTraceList.py | 43 +++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/scopesim/effects/metis_lms_trace_list.py b/scopesim/effects/metis_lms_trace_list.py index aefaf3ea..44d13c4b 100644 --- a/scopesim/effects/metis_lms_trace_list.py +++ b/scopesim/effects/metis_lms_trace_list.py @@ -235,7 +235,7 @@ def rectify_cube(self, hdulist, xi_min=None, xi_max=None, interps=None, for i, spt in enumerate(self.spectral_traces.values()): spt.wave_min = wave_min spt.wave_max = wave_max - result = spt.rectify(hdulist, interps=interps, + result = spt.rectify(inhdul, interps=interps, wave_min=wave_min, wave_max=wave_max, xi_min=xi_min, xi_max=xi_max, bin_width=dwave, diff --git a/scopesim/effects/spectral_trace_list.py b/scopesim/effects/spectral_trace_list.py index 2b4e8980..6c3612f3 100644 --- a/scopesim/effects/spectral_trace_list.py +++ b/scopesim/effects/spectral_trace_list.py @@ -348,7 +348,7 @@ def rectify_traces(self, hdulist, xi_min=None, xi_max=None, interps=None, if interps is None: logger.debug("Computing interpolation functions") - interps = make_image_interpolations(hdulist) + interps = make_image_interpolations(inhdul) pdu = fits.PrimaryHDU() pdu.header["FILETYPE"] = "Rectified spectra" @@ -358,7 +358,7 @@ def rectify_traces(self, hdulist, xi_min=None, xi_max=None, interps=None, for i, trace_id in tqdm(enumerate(self.spectral_traces, start=1), desc=" Traces", total=len(self.spectral_traces)): - hdu = self[trace_id].rectify(hdulist, + hdu = self[trace_id].rectify(inhdul, interps=interps, bin_width=bin_width, xi_min=xi_min, xi_max=xi_max, diff --git a/scopesim/tests/tests_effects/test_SpectralTraceList.py b/scopesim/tests/tests_effects/test_SpectralTraceList.py index ca634a08..086d52f5 100644 --- a/scopesim/tests/tests_effects/test_SpectralTraceList.py +++ b/scopesim/tests/tests_effects/test_SpectralTraceList.py @@ -80,6 +80,49 @@ def fixture_spectral_trace_list(): # spectral_trace_list.rectify_traces(hdulist) +class TestRectifyTracesInput: + def test_rectify_traces_opens_filename_before_use( + self, spectral_trace_list, tmp_path, monkeypatch): + """rectify_traces accepts a filename, but passed the raw string on + to make_image_interpolations and SpectralTrace.rectify instead of + the opened HDUList.""" + import numpy as np + from astropy.table import Table + from scopesim.effects import spectral_trace_list as stl_mod + + readout = tmp_path / "readout.fits" + fits.HDUList([fits.PrimaryHDU(), + fits.ImageHDU(data=np.zeros((10, 10)))] + ).writeto(readout) + + # Stub the filter lookup; it needs a fully configured instrument + class FakeFilterCurve: + table = Table(data=[[1.0, 2.0], [1.0, 1.0]], + names=["wavelength", "transmission"]) + + monkeypatch.setattr(stl_mod, "FilterCurve", + lambda **kwargs: FakeFilterCurve()) + monkeypatch.setattr(stl_mod, "from_currsys", + lambda *args, **kwargs: "J") + + # Record what object each trace's rectify() receives + received = [] + + def fake_rectify(self, hdulist, **kwargs): + received.append(hdulist) + return None + + monkeypatch.setattr(SpectralTrace, "rectify", fake_rectify) + + result = spectral_trace_list.rectify_traces( + str(readout), xi_min=-1, xi_max=1) + + assert isinstance(result, fits.HDUList) + assert received, "rectify() was never called" + assert all(isinstance(hdul, fits.HDUList) for hdul in received), \ + "traces received the filename string, not the opened HDUList" + + class TestSpectralTraceListWheel: @pytest.mark.usefixtures("no_file_error") def test_basic_init(self): From f930ee221d60f6a1c55e4d20f41e6f3ea6f0b3a5 Mon Sep 17 00:00:00 2001 From: Kieran Leschinski Date: Wed, 19 Aug 2026 15:41:10 +0200 Subject: [PATCH 2/3] Return None from map_spectra_to_focal_plane when XiLamImage fails A ValueError raised while building the XiLamImage was caught and logged, but execution then continued with the unbound local variable, turning the handled error into a NameError. Return None instead, which callers already handle (the same contract as the footprint-outside-FoV and empty-footprint paths). Adds a unit test that simulates the failure. --- scopesim/effects/spectral_trace_list_utils.py | 1 + .../test_SpectralTraceListUtils.py | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/scopesim/effects/spectral_trace_list_utils.py b/scopesim/effects/spectral_trace_list_utils.py index d523aee7..2d065f39 100644 --- a/scopesim/effects/spectral_trace_list_utils.py +++ b/scopesim/effects/spectral_trace_list_utils.py @@ -230,6 +230,7 @@ def map_spectra_to_focal_plane(self, fov): self._xilamimg = xilam # ..todo: remove or make available with a debug flag? except ValueError: logger.warning(" ---> %s gave ValueError", self.trace_id) + return None npix_xi, npix_lam = xilam.npix_xi, xilam.npix_lam xilam_wcs = xilam.wcs diff --git a/scopesim/tests/tests_effects/test_SpectralTraceListUtils.py b/scopesim/tests/tests_effects/test_SpectralTraceListUtils.py index ca2f19d2..c1ca366e 100644 --- a/scopesim/tests/tests_effects/test_SpectralTraceListUtils.py +++ b/scopesim/tests/tests_effects/test_SpectralTraceListUtils.py @@ -159,6 +159,32 @@ def test_grid_false_shape_is_preserved(self, tf2d): assert res.shape == (n_y, n_x) +class TestMapSpectraErrorPath: + def test_returns_none_when_xilamimage_fails(self, monkeypatch): + """A ValueError from XiLamImage was logged but then the unbound + variable was used anyway, raising NameError instead.""" + from scopesim.effects import spectral_trace_list_utils as stlu_mod + from scopesim.tests.mocks.py_objects import header_objects as ho + + class FailingXiLamImage: + def __init__(self, *args, **kwargs): + raise ValueError("simulated failure") + + monkeypatch.setattr(stlu_mod, "XiLamImage", FailingXiLamImage) + + spt = SpectralTrace(tlo.trace_1(), trace_id="TRACE_1", + pixel_scale=0.004, plate_scale=0.266) + + class FakeFov: + header = ho._basic_fov_header() + detector_header = header + trace_id = "TRACE_1" + meta = {"wave_min": 1.2 * u.um, "wave_max": 2.4 * u.um, + "xi_min": -1.5 * u.arcsec, "xi_max": 1.5 * u.arcsec} + + assert spt.map_spectra_to_focal_plane(FakeFov()) is None + + class MockCubeFov: """Minimal stand-in for a FieldOfView carrying a spectral cube.""" def __init__(self, n_lam=20, n_eta=3, n_xi=11): From ae1424659a08b37cf20abb979c8420fcd9ddc436 Mon Sep 17 00:00:00 2001 From: Kieran Leschinski Date: Wed, 19 Aug 2026 15:46:45 +0200 Subject: [PATCH 3/3] Stop MetisLMSSpectralTrace writing into the caller's params dict MetisLMSSpectralTraceList passes params=self.meta by reference; each trace then updated that dict with its slice-specific values (slice, aperture_id, plus any construction kwargs). After building 28 traces the list's own meta carried the values of whichever slice was built last, and traces could see each other's entries. Copy the dict at the constructor boundary. Adds a regression test asserting the caller's dict is unchanged. --- scopesim/effects/metis_lms_trace_list.py | 1 + .../tests_effects/test_MetisLMSTraceList.py | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/scopesim/effects/metis_lms_trace_list.py b/scopesim/effects/metis_lms_trace_list.py index 44d13c4b..2741f83b 100644 --- a/scopesim/effects/metis_lms_trace_list.py +++ b/scopesim/effects/metis_lms_trace_list.py @@ -301,6 +301,7 @@ class MetisLMSSpectralTrace(SpectralTrace): def __init__(self, hdulist, spslice, params, **kwargs): polyhdu = hdulist["Polynomial coefficients"] + params = dict(params) # do not modify the caller's dictionary params.update(kwargs) params["aperture_id"] = spslice params["slice"] = spslice diff --git a/scopesim/tests/tests_effects/test_MetisLMSTraceList.py b/scopesim/tests/tests_effects/test_MetisLMSTraceList.py index 3cf1dc09..23985b36 100644 --- a/scopesim/tests/tests_effects/test_MetisLMSTraceList.py +++ b/scopesim/tests/tests_effects/test_MetisLMSTraceList.py @@ -4,7 +4,9 @@ import pytest from numpy.testing import assert_allclose from astropy.io import fits -from scopesim.effects.metis_lms_trace_list import predisperser_angle +from scopesim.effects.metis_lms_trace_list import (predisperser_angle, + echelle_setting, + MetisLMSSpectralTrace) # pylint: disable=missing-class-docstring @@ -38,6 +40,26 @@ def counting_read(*args, **kwargs): assert first is second mlt._read_detector_layout.cache_clear() +class TestMetisLMSSpectralTraceInit: + def test_does_not_mutate_caller_params(self, mock_dir, monkeypatch): + """The params dict is passed by reference from the trace list's + self.meta; the trace previously wrote its slice-specific values + (slice, aperture_id, ...) back into it.""" + monkeypatch.setattr(MetisLMSSpectralTrace, "fov_grid", + lambda self: {}) + + with fits.open(mock_dir / "METIS_LMS/TRACE_LMS.fits") as hdul: + ech = echelle_setting(4.2, 18.2, hdul["WCAL"].data) + params = {"order": ech["Ord"], "echelle": ech["Echelle"], + "wavelen": 4.2} + snapshot = dict(params) + + trace = MetisLMSSpectralTrace(hdul, spslice=0, params=params) + + assert params == snapshot, \ + "constructor modified the caller's params dict" + assert trace.meta["slice"] == 0 + assert trace.meta["aperture_id"] == 0 @pytest.mark.usefixtures("patch_mock_path_metis")