From 3122b03f9716bee9439325664f18fee01c38e256 Mon Sep 17 00:00:00 2001 From: Parikshit Date: Fri, 31 Jul 2026 03:17:16 +0530 Subject: [PATCH 1/2] Normalize flip_angles in DCEAcquisitionParams.__post_init__ --- osipy/cli/runner.py | 2 +- osipy/common/types.py | 38 ++++++++++++++++++ osipy/dce/t1_mapping/vfa.py | 12 +++--- tests/unit/dce/test_t1_mapping.py | 66 +++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 7 deletions(-) diff --git a/osipy/cli/runner.py b/osipy/cli/runner.py index 8ebcdbb..1cec4cf 100644 --- a/osipy/cli/runner.py +++ b/osipy/cli/runner.py @@ -596,7 +596,7 @@ def _run_dce_from_dicom( dce_data=dce_dataset, time=time_seconds, t1_data=vfa_dataset, - flip_angles=np.asarray(vfa_flip_angles, dtype=float), + flip_angles=vfa_flip_angles, tr=vfa_tr, mask=mask, ) diff --git a/osipy/common/types.py b/osipy/common/types.py index 718baed..4961433 100644 --- a/osipy/common/types.py +++ b/osipy/common/types.py @@ -22,6 +22,8 @@ import numpy as np if TYPE_CHECKING: + from collections.abc import Sequence + from numpy.typing import NDArray @@ -179,6 +181,42 @@ class DCEAcquisitionParams(AcquisitionParams): relaxivity: float = 4.5 t1_assumed: float | None = None + def __post_init__(self) -> None: + """Normalize flip_angles into a validated list[float].""" + self.flip_angles = self._normalize_flip_angles(self.flip_angles) + + @staticmethod + def _normalize_flip_angles( + value: "Sequence[float] | NDArray[np.floating[Any]] | None", + ) -> list[float]: + """Convert flip_angles input to a validated list[float].""" + if value is None: + raise ValueError("flip_angles cannot be None") + + if isinstance(value, (int, float)): + raise ValueError( + f"flip_angles must be a sequence of floats, got a single " + f"scalar value: {value!r}" + ) + + try: + arr = np.asarray(value, dtype=float) + except (TypeError, ValueError) as exc: + raise ValueError( + f"flip_angles must be convertible to a list of floats: {exc}" + ) from exc + + if arr.ndim != 1: + raise ValueError( + f"flip_angles must be a 1D sequence, got shape {arr.shape}" + ) + + if arr.size > 0 and not np.all(np.isfinite(arr)): + raise ValueError("flip_angles contains NaN or infinite values") + + result: list[float] = arr.tolist() + return result + @dataclass class DSCAcquisitionParams(AcquisitionParams): diff --git a/osipy/dce/t1_mapping/vfa.py b/osipy/dce/t1_mapping/vfa.py index 3eb1e75..b4f6bf0 100644 --- a/osipy/dce/t1_mapping/vfa.py +++ b/osipy/dce/t1_mapping/vfa.py @@ -149,8 +149,10 @@ def _compute_t1_vfa_impl( raise DataValidationError(msg) params = dataset.acquisition_params - if not params.flip_angles: - msg = "VFA T1 mapping requires flip_angles in acquisition_params" + flip_angles = np.asarray(params.flip_angles) + + if flip_angles.size == 0: + msg = "VFA T1 mapping requires at least one flip angle" raise DataValidationError(msg) if params.tr is None: @@ -158,7 +160,7 @@ def _compute_t1_vfa_impl( raise DataValidationError(msg) xp = get_array_module(dataset.data) - flip_angles = xp.asarray(params.flip_angles) + flip_angles = xp.asarray(flip_angles) tr = params.tr # Check data dimensions match flip angles @@ -348,12 +350,10 @@ def compute_t1_vfa( msg = "Either dataset or (signal, flip_angles, tr) must be provided" raise DataValidationError(msg) - flip_angles_array = np.asarray(flip_angles) - # Create acquisition params acq_params = DCEAcquisitionParams( tr=tr, - flip_angles=list(flip_angles_array), + flip_angles=flip_angles, # type: ignore[arg-type] # normalized in __post_init__ ) # Create mask if not provided diff --git a/tests/unit/dce/test_t1_mapping.py b/tests/unit/dce/test_t1_mapping.py index 0f9bce6..955be6f 100644 --- a/tests/unit/dce/test_t1_mapping.py +++ b/tests/unit/dce/test_t1_mapping.py @@ -13,6 +13,7 @@ from osipy.common.types import DCEAcquisitionParams, Modality from osipy.dce.t1_mapping.binding import BoundLookLockerModel, BoundSPGRModel from osipy.dce.t1_mapping.models import LookLockerSignalModel, SPGRSignalModel +from osipy.dce.t1_mapping.vfa import compute_t1_vfa # --------------------------------------------------------------------------- # Helpers for synthetic data generation @@ -171,6 +172,62 @@ def test_look_locker_params(self) -> None: assert params.te == 2.0 +class TestFlipAnglesNormalization: + """Tests for DCEAcquisitionParams.__post_init__ flip_angles handling.""" + + def test_accepts_list(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=[2.0, 5.0, 10.0, 15.0]) + assert p.flip_angles == [2.0, 5.0, 10.0, 15.0] + assert isinstance(p.flip_angles, list) + + def test_accepts_numpy_array(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=np.array([2.0, 5.0, 10.0, 15.0])) + assert p.flip_angles == [2.0, 5.0, 10.0, 15.0] + assert isinstance(p.flip_angles, list) + + def test_accepts_tuple(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=(2, 5, 10, 15)) + assert p.flip_angles == [2.0, 5.0, 10.0, 15.0] + + def test_accepts_int_list(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=[2, 5, 10, 15]) + assert all(isinstance(x, float) for x in p.flip_angles) + + def test_allows_empty_list_for_fixed_t1_workflows(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=[], t1_assumed=1400.0) + assert p.flip_angles == [] + + def test_allows_empty_array_for_fixed_t1_workflows(self) -> None: + p = DCEAcquisitionParams(tr=5.0, flip_angles=np.array([]), t1_assumed=1400.0) + assert p.flip_angles == [] + + def test_rejects_none(self) -> None: + with pytest.raises(ValueError, match="cannot be None"): + DCEAcquisitionParams(tr=5.0, flip_angles=None) + + def test_rejects_scalar(self) -> None: + with pytest.raises(ValueError, match="single scalar"): + DCEAcquisitionParams(tr=5.0, flip_angles=5.0) + + def test_rejects_non_numeric(self) -> None: + with pytest.raises(ValueError): + DCEAcquisitionParams(tr=5.0, flip_angles=["a", "b", "c"]) + + def test_rejects_nan(self) -> None: + with pytest.raises(ValueError, match="NaN or infinite"): + DCEAcquisitionParams(tr=5.0, flip_angles=[2.0, float("nan"), 10.0]) + + def test_rejects_inf(self) -> None: + with pytest.raises(ValueError, match="NaN or infinite"): + DCEAcquisitionParams(tr=5.0, flip_angles=[2.0, float("inf"), 10.0]) + + def test_rejects_2d_array(self) -> None: + with pytest.raises(ValueError, match="1D sequence"): + DCEAcquisitionParams( + tr=5.0, flip_angles=np.array([[2.0, 5.0], [10.0, 15.0]]) + ) + + class TestSyntheticT1Data: """Tests for generating synthetic T1 data.""" @@ -477,6 +534,15 @@ def test_vfa_invalid_method_raises(self) -> None: with pytest.raises(DataValidationError, match="Unknown VFA method"): compute_t1_vfa(dataset, method="bogus") + def test_vfa_rejects_empty_flip_angles(self) -> None: + """compute_t1_vfa rejects empty flip_angles even though the + dataclass itself allows it (for fixed-T1/t1_assumed workflows).""" + dataset = _make_vfa_dataset(1000.0, 100.0, [2.0, 5.0, 10.0], 5.0) + dataset.acquisition_params.flip_angles = [] + + with pytest.raises(DataValidationError, match="at least one flip angle"): + compute_t1_vfa(dataset, method="linear") + class TestLookLockerFitting: """End-to-end Look-Locker T1 fitting tests.""" From 5d858962ba011066b606fe9e6dcfe6cdc611b4c7 Mon Sep 17 00:00:00 2001 From: Parikshit Date: Wed, 5 Aug 2026 02:59:02 +0530 Subject: [PATCH 2/2] Address review: remove redundant asarray, use len() for flip_angles check --- osipy/dce/t1_mapping/vfa.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osipy/dce/t1_mapping/vfa.py b/osipy/dce/t1_mapping/vfa.py index b4f6bf0..65171cf 100644 --- a/osipy/dce/t1_mapping/vfa.py +++ b/osipy/dce/t1_mapping/vfa.py @@ -149,9 +149,8 @@ def _compute_t1_vfa_impl( raise DataValidationError(msg) params = dataset.acquisition_params - flip_angles = np.asarray(params.flip_angles) - if flip_angles.size == 0: + if len(params.flip_angles) == 0: msg = "VFA T1 mapping requires at least one flip angle" raise DataValidationError(msg) @@ -160,7 +159,7 @@ def _compute_t1_vfa_impl( raise DataValidationError(msg) xp = get_array_module(dataset.data) - flip_angles = xp.asarray(flip_angles) + flip_angles = xp.asarray(params.flip_angles) tr = params.tr # Check data dimensions match flip angles