From 9261d6255dc85362a591789ecdc68e75fecc540c Mon Sep 17 00:00:00 2001 From: Roy Smart Date: Wed, 19 Aug 2026 13:08:36 -0600 Subject: [PATCH] Raise a clear error when `Camera.gain` is `None` `Camera.gain` defaults to `None` and, unlike `Camera.sensor`, nothing ever resolves it. `AbstractCamera.dn_to_electrons` then computed `None * a`, which silently produced a `TapData` whose `outputs` was `None` instead of raising. The failure only surfaced later in `SensorData.from_taps` as `TypeError: NoneType object is not subscriptable`, which points at the wrong place entirely. Since `msfc_ccd.fits.open` constructs a default `Camera()`, every image loaded the documented way hit this. `dn_to_electrons` now raises a `ValueError` naming the missing parameter. A measured default was considered instead, but there is no measured gain in this repository yet and the gain is tap-dependent, so a single hardcoded value would be wrong. Added tests covering `.electrons` on an image from `msfc_ccd.fits.open`, both with an explicit gain and with the default camera, since nothing exercised that path. Co-Authored-By: Claude Opus 5 --- msfc_ccd/_cameras.py | 31 ++++++++++++++- msfc_ccd/_tests/test_cameras.py | 26 +++++++++++++ msfc_ccd/_tests/test_fits.py | 67 +++++++++++++++++++++++++++------ msfc_ccd/fits.py | 3 ++ 4 files changed, 113 insertions(+), 14 deletions(-) diff --git a/msfc_ccd/_cameras.py b/msfc_ccd/_cameras.py index c6f6f0f..824c2b9 100644 --- a/msfc_ccd/_cameras.py +++ b/msfc_ccd/_cameras.py @@ -119,8 +119,31 @@ def dn_to_electrons( self, a: u.Quantity | na.AbstractArray, ) -> na.AbstractArray: - """Convert an array from DN to electrons by multiplying by :attr:`gain`.""" - return self.gain * a + """ + Convert an array from DN to electrons by multiplying by :attr:`gain`. + + Parameters + ---------- + a + The array, in units of DN, to convert into units of electrons. + + Raises + ------ + ValueError + If :attr:`gain` is :obj:`None`, since there is no measured default + gain for these cameras. + """ + gain = self.gain + + if gain is None: + raise ValueError( + "`gain` is `None`, so this camera cannot convert DN into " + "electrons. Measure the gain of this camera and provide it " + "explicitly, for example " + "`msfc_ccd.Camera(gain=2.5 * u.electron / u.DN)`." + ) + + return gain * a @dataclasses.dataclass(repr=False) @@ -149,6 +172,10 @@ class Camera( This is usually tap-dependent and contains :attr:`axis_tap_x` and :attr:`axis_tap_y` dimensions. + + There is no measured default, so this must be provided explicitly before + :meth:`msfc_ccd.abc.AbstractCamera.dn_to_electrons` (and therefore + :attr:`msfc_ccd.abc.AbstractCameraData.electrons`) can be used. """ bits_adc: int = 16 diff --git a/msfc_ccd/_tests/test_cameras.py b/msfc_ccd/_tests/test_cameras.py index e8d671d..a86d1dd 100644 --- a/msfc_ccd/_tests/test_cameras.py +++ b/msfc_ccd/_tests/test_cameras.py @@ -9,6 +9,26 @@ class AbstractTestAbstractCamera( AbstractTestPrintable, ): + def test_gain( + self, + a: msfc_ccd.abc.AbstractCamera, + ): + result = a.gain + if result is not None: + assert na.unit(result).is_equivalent(u.electron / u.DN) + + def test_dn_to_electrons( + self, + a: msfc_ccd.abc.AbstractCamera, + ): + b = 100 * u.DN + if a.gain is None: + with pytest.raises(ValueError, match="`gain` is `None`"): + a.dn_to_electrons(b) + else: + result = a.dn_to_electrons(b) + assert na.unit(result).is_equivalent(u.electron) + @pytest.mark.parametrize("value", [1, 10]) def test_calibrate_timedelta_exposure( self, @@ -59,6 +79,12 @@ def test_calibrate_temperature_adc_234( argnames="a", argvalues=[ msfc_ccd.Camera(), + msfc_ccd.Camera( + gain=na.ScalarArray( + ndarray=[[2.5, 2.6], [2.7, 2.8]] * u.electron / u.DN, + axes=("tap_x", "tap_y"), + ), + ), ], ) class TestCamera( diff --git a/msfc_ccd/_tests/test_fits.py b/msfc_ccd/_tests/test_fits.py index 2ed7c89..a88dca3 100644 --- a/msfc_ccd/_tests/test_fits.py +++ b/msfc_ccd/_tests/test_fits.py @@ -1,26 +1,69 @@ import pytest import pathlib import numpy as np +import astropy.units as u import named_arrays as na import msfc_ccd +_paths = [ + msfc_ccd.samples.path_fe55_esis1, + na.ScalarArray( + ndarray=np.array( + [ + msfc_ccd.samples.path_fe55_esis1, + msfc_ccd.samples.path_fe55_esis3, + ] + ), + axes="time", + ), +] + +_camera = msfc_ccd.Camera( + gain=na.ScalarArray( + ndarray=[[2.5, 2.6], [2.7, 2.8]] * u.electron / u.DN, + axes=("tap_x", "tap_y"), + ), +) + @pytest.mark.parametrize( argnames="path", - argvalues=[ - msfc_ccd.samples.path_fe55_esis1, - na.ScalarArray( - ndarray=np.array( - [ - msfc_ccd.samples.path_fe55_esis1, - msfc_ccd.samples.path_fe55_esis3, - ] - ), - axes="time", - ), - ], + argvalues=_paths, ) def test_open(path: str | pathlib.Path | na.AbstractScalarArray): result = msfc_ccd.fits.open(path) assert isinstance(result, msfc_ccd.SensorData) assert result.outputs.sum() != 0 + + +@pytest.mark.parametrize( + argnames="path", + argvalues=_paths, +) +def test_open_electrons(path: str | pathlib.Path | na.AbstractScalarArray): + image = msfc_ccd.fits.open(path, camera=_camera) + + taps = image.taps.electrons + assert isinstance(taps, msfc_ccd.TapData) + assert na.unit(taps.outputs).is_equivalent(u.electron) + assert taps.outputs.sum() != 0 * u.electron + + result = image.electrons + assert isinstance(result, msfc_ccd.SensorData) + assert na.unit(result.outputs).is_equivalent(u.electron) + assert result.outputs.sum() != 0 * u.electron + + +@pytest.mark.parametrize( + argnames="path", + argvalues=_paths, +) +def test_open_electrons_default_camera( + path: str | pathlib.Path | na.AbstractScalarArray, +): + """The default camera has no gain, so ``electrons`` should say so plainly.""" + image = msfc_ccd.fits.open(path) + with pytest.raises(ValueError, match="`gain` is `None`"): + image.taps.electrons + with pytest.raises(ValueError, match="`gain` is `None`"): + image.electrons diff --git a/msfc_ccd/fits.py b/msfc_ccd/fits.py index 9546a95..160f80a 100644 --- a/msfc_ccd/fits.py +++ b/msfc_ccd/fits.py @@ -29,6 +29,9 @@ def open( A model of the camera used to capture the images being loaded. If :obj:`None` (the default), the :class:`msfc_ccd.Camera` will be used. + Note that the default camera has no :attr:`msfc_ccd.Camera.gain`, + so a camera with an explicit gain must be provided to use + :attr:`msfc_ccd.SensorData.electrons` on the result. axis_x The name of the logical axis representing the horizontal dimension of the images.