From c33a47928f9b478cae79c981e62f7f3c8c726e24 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Mon, 6 Jul 2026 16:13:57 +0200 Subject: [PATCH] Centralize Q-index validation via verify_Q_index Replace the AnalysisBase._verify_Q_index method and the various inline Q_index type/bounds checks scattered across the model hierarchy with the shared verify_Q_index helper (added in the previous PR). Callers now delegate validation to a single function with consistent error messages and behavior (TypeError for non-int, IndexError for negative/out-of-range, upper-bound deferred when Q is None). Migrated: Analysis, Analysis1d, AnalysisBase, Experiment, ModelBase, InstrumentModel, DiffusionModelBase and DeltaLorentz. Tests updated to the unified messages. Split out of the larger unit-system branch. All unit tests pass. Co-Authored-By: Claude Opus 4.8 --- src/easydynamics/analysis/analysis.py | 12 ++--- src/easydynamics/analysis/analysis1d.py | 8 ++-- src/easydynamics/analysis/analysis_base.py | 31 ------------ src/easydynamics/experiment/experiment.py | 13 +---- .../diffusion_model/delta_lorentz.py | 31 ++---------- .../diffusion_model/diffusion_model_base.py | 47 ++----------------- .../sample_model/instrument_model.py | 42 ++--------------- src/easydynamics/sample_model/model_base.py | 32 ++----------- .../easydynamics/analysis/test_analysis.py | 4 +- .../easydynamics/analysis/test_analysis1d.py | 4 +- .../analysis/test_analysis_base.py | 18 ------- .../experiment/test_experiment.py | 12 +++-- .../diffusion_model/test_delta_lorentz.py | 18 +++---- .../test_diffusion_model_base.py | 33 ++++++++----- .../sample_model/test_model_base.py | 2 +- 15 files changed, 71 insertions(+), 236 deletions(-) diff --git a/src/easydynamics/analysis/analysis.py b/src/easydynamics/analysis/analysis.py index 7291ea917..712169669 100644 --- a/src/easydynamics/analysis/analysis.py +++ b/src/easydynamics/analysis/analysis.py @@ -20,6 +20,7 @@ from easydynamics.settings.detailed_balance_settings import DetailedBalanceSettings from easydynamics.utils.plotting import slicerplot_with_residuals from easydynamics.utils.utils import _in_notebook +from easydynamics.utils.utils import verify_Q_index class Analysis(AnalysisBase): @@ -253,7 +254,7 @@ def calculate( if Q_index is None: return [analysis.calculate(energy=energy) for analysis in self.analysis_list] - Q_index = self._verify_Q_index(Q_index) + verify_Q_index(Q_index=Q_index, Q=self.Q) return self.analysis_list[Q_index].calculate(energy=energy) def fit( @@ -291,7 +292,7 @@ def fit( 'No Q values available for fitting. Please check the experiment data.' ) - Q_index = self._verify_Q_index(Q_index) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if fit_method == 'independent': if Q_index is not None: @@ -347,9 +348,8 @@ def plot_data_and_model( InteractiveFigure A Plopp InteractiveFigure containing the plot of the data and model. """ - + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if Q_index is not None: - Q_index = self._verify_Q_index(Q_index) return self.analysis_list[Q_index].plot_data_and_model( plot_components=plot_components, add_background=add_background, @@ -627,8 +627,8 @@ def fix_energy_offset(self, Q_index: int | None = None) -> None: Index of the Q value to fix the energy offset for. If None, fixes the energy offset for all Q values. """ + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if Q_index is not None: - Q_index = self._verify_Q_index(Q_index) self.analysis_list[Q_index].fix_energy_offset() else: for analysis in self.analysis_list: @@ -645,8 +645,8 @@ def free_energy_offset(self, Q_index: int | None = None) -> None: Index of the Q value to free the energy offset for. If None, frees the energy offset for all Q values. """ + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if Q_index is not None: - Q_index = self._verify_Q_index(Q_index) self.analysis_list[Q_index].free_energy_offset() else: for analysis in self.analysis_list: diff --git a/src/easydynamics/analysis/analysis1d.py b/src/easydynamics/analysis/analysis1d.py index 341103e16..01fa826f5 100644 --- a/src/easydynamics/analysis/analysis1d.py +++ b/src/easydynamics/analysis/analysis1d.py @@ -22,6 +22,7 @@ from easydynamics.settings.detailed_balance_settings import DetailedBalanceSettings from easydynamics.utils.detailed_balance import detailed_balance_factor from easydynamics.utils.plotting import slicerplot_with_residuals +from easydynamics.utils.utils import verify_Q_index class Analysis1d(AnalysisBase): @@ -119,7 +120,8 @@ def __init__( extra_parameters=extra_parameters, ) - self._Q_index = self._verify_Q_index(Q_index) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) + self._Q_index = Q_index if self._Q_index is not None and self.experiment is not None: masked_energy = self.experiment.get_masked_energy(Q_index=self._Q_index) @@ -158,8 +160,8 @@ def Q_index(self, value: int | None) -> None: value : int | None The Q index. """ - - self._Q_index = self._verify_Q_index(value) + verify_Q_index(Q_index=value, Q=self.Q, allow_none=True) + self._Q_index = value self._on_Q_index_changed() ############# diff --git a/src/easydynamics/analysis/analysis_base.py b/src/easydynamics/analysis/analysis_base.py index 598daea67..48b4abc89 100644 --- a/src/easydynamics/analysis/analysis_base.py +++ b/src/easydynamics/analysis/analysis_base.py @@ -494,37 +494,6 @@ def _on_convolution_settings_changed(self) -> None: For subclasses that implement convolution, this method can be overridden """ - def _verify_Q_index(self, Q_index: int | None) -> int | None: - """ - Verify that the Q index is valid. - - Parameters - ---------- - Q_index : int | None - The Q index to verify. - - Raises - ------ - TypeError - If Q_index is not an integer or None. - IndexError - If the Q index is not valid. - - Returns - ------- - int | None - The verified Q index. - """ - if Q_index is None: - return None - - if not isinstance(Q_index, int): - raise TypeError('Q_index must be an integer or None.') - - if Q_index < 0 or (self.Q is not None and Q_index >= len(self.Q)): - raise IndexError('Q_index must be a valid index for the Q values.') - return Q_index - def _verify_energy(self, energy: sc.Variable | None) -> sc.Variable | None: """ Verify that the provided energy is the correct type. diff --git a/src/easydynamics/experiment/experiment.py b/src/easydynamics/experiment/experiment.py index a2fb04fe4..a27dc2b66 100644 --- a/src/easydynamics/experiment/experiment.py +++ b/src/easydynamics/experiment/experiment.py @@ -13,6 +13,7 @@ from easydynamics.base_classes.easydynamics_base import EasyDynamicsBase from easydynamics.utils.utils import _in_notebook +from easydynamics.utils.utils import verify_Q_index class Experiment(EasyDynamicsBase): @@ -239,11 +240,6 @@ def get_masked_energy(self, Q_index: int) -> sc.Variable | None: Q_index : int The Q index to get the masked energy values for. - Raises - ------ - IndexError - If Q_index is not a valid index for the Q values. - Returns ------- sc.Variable | None @@ -252,12 +248,7 @@ def get_masked_energy(self, Q_index: int) -> sc.Variable | None: if self.binned_data is None: return None - if ( - not isinstance(Q_index, int) - or Q_index < 0 - or (self.Q is not None and Q_index >= len(self.Q)) - ): - raise IndexError('Q_index must be a valid index for the Q values.') + verify_Q_index(Q_index, self.Q) energy = self.binned_data.coords['energy'] _, _, _, mask = self._extract_x_y_weights_only_finite(Q_index=Q_index) diff --git a/src/easydynamics/sample_model/diffusion_model/delta_lorentz.py b/src/easydynamics/sample_model/diffusion_model/delta_lorentz.py index 1e709dbc4..0ee8861b5 100644 --- a/src/easydynamics/sample_model/diffusion_model/delta_lorentz.py +++ b/src/easydynamics/sample_model/diffusion_model/delta_lorentz.py @@ -13,6 +13,7 @@ from easydynamics.sample_model.diffusion_model.diffusion_model_base import DiffusionModelBase from easydynamics.utils.utils import Numeric from easydynamics.utils.utils import Q_type +from easydynamics.utils.utils import verify_Q_index MINIMUM_WIDTH = 1e-10 # To avoid division by zero @@ -587,22 +588,9 @@ def get_independent_variables(self, Q_index: int | None = None) -> list[Paramete ------- list[Parameter] List of independent variables in the model. - - Raises - ------ - ValueError - If Q_index is not None and is not a valid index for the Q values in the model. """ - if Q_index is not None and ( - not isinstance(Q_index, int) - or Q_index < 0 - or Q_index >= len(self._component_collections) - ): - raise ValueError( - f'Q_index must be an integer between 0 and ' - f'{len(self._component_collections) - 1}, or None.' - ) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) variables = [] if self._allow_Q_variation['A_0'] is True: @@ -629,22 +617,9 @@ def get_all_variables(self, Q_index: int | None = None) -> list[DescriptorNumber ------- list[DescriptorNumber] List of all variables in the model. - - Raises - ------ - ValueError - If Q_index is not None and is not a valid index for the Q values in the model. """ - if Q_index is not None and ( - not isinstance(Q_index, int) - or Q_index < 0 - or Q_index >= len(self._component_collections) - ): - raise ValueError( - f'Q_index must be an integer between 0 and ' - f'{len(self._component_collections) - 1}, or None.' - ) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) variables = self.get_global_variables() variables.extend(self.get_independent_variables(Q_index=Q_index)) diff --git a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py index 7d752b826..de28beacc 100644 --- a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py +++ b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py @@ -12,6 +12,7 @@ from easydynamics.utils.utils import Numeric from easydynamics.utils.utils import Q_type from easydynamics.utils.utils import _validate_and_convert_Q +from easydynamics.utils.utils import verify_Q_index class DiffusionModelBase(EasyDynamicsModelBase): @@ -305,21 +306,8 @@ def get_independent_variables(self, Q_index: int | None = None) -> list[Paramete ------- list[Parameter] List of independent variables in the model. - - Raises - ------ - ValueError - If Q_index is not None and is not a valid index for the Q values in the model. """ - if Q_index is not None and ( - not isinstance(Q_index, int) - or Q_index < 0 - or Q_index >= len(self._component_collections) - ): - raise ValueError( - f'Q_index must be an integer between 0 and ' - f'{max(len(self._component_collections) - 1, 0)}, or None.' - ) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) return [] @@ -337,22 +325,8 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: ------- list[Parameter] A list of all Parameters from the diffusion model. - - Raises - ------ - ValueError - If Q_index is out of bounds for the number of ComponentCollections. """ - - if Q_index is not None and ( - not isinstance(Q_index, int) - or Q_index < 0 - or Q_index >= len(self._component_collections) - ): - raise ValueError( - f'Q_index must be an integer between 0 and ' - f'{max(len(self._component_collections) - 1, 0)}, or None.' - ) + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) variables = self.get_global_variables() variables.extend(self.get_independent_variables(Q_index)) @@ -464,29 +438,16 @@ def get_component_collections( The index of the desired ComponentCollection. If None, all ComponentCollections are returned. - Raises - ------ - TypeError - If Q_index is not an int. - IndexError - If Q_index is out of bounds for the number of ComponentCollections. - Returns ------- ComponentCollection | list[ComponentCollection] The ComponentCollection at the specified Q index. If Q_index is None, a list of all ComponentCollections is returned. """ + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if Q_index is None: return self._component_collections - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int, got {type(Q_index).__name__}') - if Q_index < 0 or Q_index >= len(self._component_collections): - raise IndexError( - f'Q_index {Q_index} is out of bounds for component collections ' - f'of length {len(self._component_collections)}' - ) return self._component_collections[Q_index] # ------------------------------------------------------------------ diff --git a/src/easydynamics/sample_model/instrument_model.py b/src/easydynamics/sample_model/instrument_model.py index 24c3f1719..4ff3b4f30 100644 --- a/src/easydynamics/sample_model/instrument_model.py +++ b/src/easydynamics/sample_model/instrument_model.py @@ -15,6 +15,7 @@ from easydynamics.utils.utils import Q_type from easydynamics.utils.utils import _validate_and_convert_Q from easydynamics.utils.utils import _validate_unit +from easydynamics.utils.utils import verify_Q_index class InstrumentModel(NewBase): @@ -395,13 +396,6 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: The index of the Q value to get variables for. If None, get variables for all Q values. - Raises - ------ - TypeError - If Q_index is not an int or None. - IndexError - If Q_index is out of bounds for the Q values in the InstrumentModel. - Returns ------- list[Parameter] @@ -413,15 +407,10 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: return [] self._ensure_energy_offsets_current() + verify_Q_index(Q_index=Q_index, Q=self._Q, allow_none=True) if Q_index is None: variables = [self._energy_offsets[i] for i in range(len(self._Q))] else: - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') - if Q_index < 0 or Q_index >= len(self._Q): - raise IndexError( - f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}' - ) variables = [self._energy_offsets[Q_index]] variables.extend(self._background_model.get_all_variables(Q_index=Q_index)) @@ -458,10 +447,6 @@ def get_energy_offset( ------ ValueError If no Q values are set in the InstrumentModel. - IndexError - If Q_index is out of bounds. - TypeError - If Q_index is not an int or None. Returns ------- @@ -473,15 +458,10 @@ def get_energy_offset( raise ValueError('No Q values are set in the InstrumentModel.') self._ensure_energy_offsets_current() + verify_Q_index(Q_index=Q_index, Q=self._Q, allow_none=True) if Q_index is None: return self._energy_offsets - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') - - if Q_index < 0 or Q_index >= len(self._Q): - raise IndexError(f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}') - return self._energy_offsets[Q_index] def fix_energy_offset(self, Q_index: int | None = None) -> None: @@ -531,27 +511,13 @@ def _fix_or_free_energy_offset(self, Q_index: int | None = None, fixed: bool = T energy offsets for all Q values. fixed : bool, default=True Whether to fix (True) or free (False) the energy offset. - - Raises - ------ - TypeError - If Q_index is not an int or None. - IndexError - If Q_index is out of bounds for the Q values in the InstrumentModel. """ - self._ensure_energy_offsets_current() + verify_Q_index(Q_index=Q_index, Q=self._Q, allow_none=True) if Q_index is None: for offset in self._energy_offsets: offset.fixed = fixed else: - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') - - if Q_index < 0 or Q_index >= len(self._Q): - raise IndexError( - f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}' - ) self._energy_offsets[Q_index].fixed = fixed def _ensure_energy_offsets_current(self) -> None: diff --git a/src/easydynamics/sample_model/model_base.py b/src/easydynamics/sample_model/model_base.py index a2f1d7765..288d42be5 100644 --- a/src/easydynamics/sample_model/model_base.py +++ b/src/easydynamics/sample_model/model_base.py @@ -13,6 +13,7 @@ from easydynamics.utils.utils import Numeric from easydynamics.utils.utils import Q_type from easydynamics.utils.utils import _validate_and_convert_Q +from easydynamics.utils.utils import verify_Q_index class ModelBase(EasyDynamicsModelBase): @@ -314,21 +315,14 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: If None, get variables for all ComponentCollections. If int, get variables for the ComponentCollection at this index. - Raises - ------ - TypeError - If Q_index is not an int or None. - IndexError - If Q_index is out of bounds for the number of ComponentCollections. - Returns ------- list[Parameter] A list of all Parameters and Descriptors from the ComponentCollections in the ModelBase. """ - self._ensure_component_collections_current() + verify_Q_index(Q_index=Q_index, Q=self.Q, allow_none=True) if Q_index is None: all_vars = [ var @@ -336,13 +330,6 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: for var in collection.get_all_variables() ] else: - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') - if Q_index < 0 or Q_index >= len(self._component_collections): - raise IndexError( - f'Q_index {Q_index} is out of bounds for component collections ' - f'of length {len(self._component_collections)}' - ) all_vars = self._component_collections[Q_index].get_all_variables() return all_vars @@ -355,26 +342,13 @@ def get_component_collection(self, Q_index: int) -> ComponentCollection: Q_index : int The index of the desired ComponentCollection. - Raises - ------ - TypeError - If Q_index is not an int. - IndexError - If Q_index is out of bounds for the number of ComponentCollections. - Returns ------- ComponentCollection The ComponentCollection at the given Q index. """ self._ensure_component_collections_current() - if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int, got {type(Q_index).__name__}') - if Q_index < 0 or Q_index >= len(self._component_collections): - raise IndexError( - f'Q_index {Q_index} is out of bounds for component collections ' - f'of length {len(self._component_collections)}' - ) + verify_Q_index(Q_index=Q_index, Q=self.Q) return self._component_collections[Q_index] def normalize_area(self) -> None: diff --git a/tests/unit/easydynamics/analysis/test_analysis.py b/tests/unit/easydynamics/analysis/test_analysis.py index 516b718ef..62e79de72 100644 --- a/tests/unit/easydynamics/analysis/test_analysis.py +++ b/tests/unit/easydynamics/analysis/test_analysis.py @@ -152,7 +152,7 @@ def test_calculate_with_invalid_Q_index(self, analysis): # WHEN / THEN / EXPECT with pytest.raises( IndexError, - match='must be a valid index', + match='Q_index 3 is out of bounds', ): analysis.calculate(Q_index=3) @@ -757,7 +757,7 @@ def test_fit_single_Q_invalid_Q_index(self, analysis): # WHEN / THEN / EXPECT with pytest.raises( IndexError, - match='must be a valid index', + match='Q_index 3 is out of bounds', ): analysis.fit(Q_index=3) diff --git a/tests/unit/easydynamics/analysis/test_analysis1d.py b/tests/unit/easydynamics/analysis/test_analysis1d.py index 5bd571634..58e090a58 100644 --- a/tests/unit/easydynamics/analysis/test_analysis1d.py +++ b/tests/unit/easydynamics/analysis/test_analysis1d.py @@ -75,8 +75,8 @@ def test_Q_index_setter(self, analysis1d): @pytest.mark.parametrize( 'invalid_Q_index, expected_exception, expected_message', [ - (-1, IndexError, 'Q_index must be'), - (10, IndexError, 'Q_index must be'), + (-1, IndexError, 'Q_index must be non-negative'), + (10, IndexError, 'Q_index 10 is out of bounds'), ('invalid', TypeError, 'Q_index must be '), (np.nan, TypeError, 'Q_index must be '), ([1, 2], TypeError, 'Q_index must be '), diff --git a/tests/unit/easydynamics/analysis/test_analysis_base.py b/tests/unit/easydynamics/analysis/test_analysis_base.py index 7507b0b35..0abd7e1fc 100644 --- a/tests/unit/easydynamics/analysis/test_analysis_base.py +++ b/tests/unit/easydynamics/analysis/test_analysis_base.py @@ -532,24 +532,6 @@ def test_on_instrument_model_changed_updates_Q(self, analysis_base): analysis_base._on_instrument_model_changed() np.testing.assert_array_equal(analysis_base.instrument_model.Q, fake_Q) - def test_verify_Q_index_valid(self, analysis_base): - # WHEN - valid_Q_index = 0 - - # THEN - result = analysis_base._verify_Q_index(valid_Q_index) - - # EXPECT - assert result == valid_Q_index - - def test_verify_Q_index_invalid(self, analysis_base): - # WHEN - invalid_Q_index = -1 - - # THEN / EXPECT - with pytest.raises(IndexError, match='Q_index must be a valid index'): - analysis_base._verify_Q_index(invalid_Q_index) - def test_repr(self, analysis_base): # WHEN repr_str = repr(analysis_base) diff --git a/tests/unit/easydynamics/experiment/test_experiment.py b/tests/unit/easydynamics/experiment/test_experiment.py index 7ffc83c87..6201bbc05 100644 --- a/tests/unit/easydynamics/experiment/test_experiment.py +++ b/tests/unit/easydynamics/experiment/test_experiment.py @@ -319,15 +319,21 @@ def test_get_masked_energy_no_data_returns_None(self): @pytest.mark.parametrize( 'Q_index', - [-1, 100, 'not an index'], - ids=['negative_index', 'out_of_bounds_index', 'invalid_type'], + [-1, 100], + ids=['negative_index', 'out_of_bounds_index'], ) def test_get_masked_energy_invalid_Q_index_raises(self, experiment_with_data, Q_index): - "Test getting masked energy raises IndexError when Q index is invalid" + "Test getting masked energy raises IndexError when Q index is out of range" # WHEN THEN EXPECT with pytest.raises(IndexError): experiment_with_data.get_masked_energy(Q_index=Q_index) + def test_get_masked_energy_invalid_type_raises(self, experiment_with_data): + "Test getting masked energy raises TypeError when Q index is not an integer" + # WHEN THEN EXPECT + with pytest.raises(TypeError): + experiment_with_data.get_masked_energy(Q_index='not an index') + ############## # test plotting ############## diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_delta_lorentz.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_delta_lorentz.py index b3740db66..bcfc704ed 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_delta_lorentz.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_delta_lorentz.py @@ -504,11 +504,11 @@ def test_create_component_collections_with_no_Q_variation( assert 'A_0' in collection[1].area.dependency_expression @pytest.mark.parametrize( - 'Q_index', + ('Q_index', 'expected_exception', 'expected_message'), [ - -1, - 100, - 'string', + (-1, IndexError, r'Q_index must be non-negative'), + (100, IndexError, r'Q_index 100 is out of bounds'), + ('string', TypeError, r'Q_index must be an int or None, got str'), ], ids=[ 'negative index', @@ -516,9 +516,11 @@ def test_create_component_collections_with_no_Q_variation( 'non-integer index', ], ) - def test_get_independent_variables_raises(self, delta_lorentz_model_with_Q, Q_index): + def test_get_independent_variables_raises( + self, delta_lorentz_model_with_Q, Q_index, expected_exception, expected_message + ): # WHEN THEN EXPECT - with pytest.raises(ValueError, match=r'Q_index must be an integer between 0 and'): + with pytest.raises(expected_exception, match=expected_message): delta_lorentz_model_with_Q.get_independent_variables(Q_index=Q_index) def test_get_all_variables_no_Q_index(self, delta_lorentz_model_with_Q): @@ -578,10 +580,10 @@ def test_get_all_variables_with_Q_index_no_Q_variation( def test_get_all_variables_invalid_Q_index(self, delta_lorentz_model_with_Q): # WHEN THEN EXPECT - with pytest.raises(ValueError, match='Q_index must be an integer between 0 and'): + with pytest.raises(IndexError, match='Q_index must be non-negative'): delta_lorentz_model_with_Q.get_all_variables(Q_index=-1) - with pytest.raises(ValueError, match='Q_index must be an integer between 0 and'): + with pytest.raises(IndexError, match=r'Q_index \d+ is out of bounds'): delta_lorentz_model_with_Q.get_all_variables(Q_index=len(delta_lorentz_model_with_Q.Q)) @pytest.mark.parametrize( diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model_base.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model_base.py index 3364e5104..27a479496 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model_base.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model_base.py @@ -163,29 +163,34 @@ def test_get_independent_variables(self, diffusion_model): assert independent_vars == [] @pytest.mark.parametrize( - 'Q_index', + ('Q_index', 'expected_exception', 'expected_message'), [ - -1, - 100, - 'string', + (-1, IndexError, 'Q_index must be non-negative'), + ('string', TypeError, 'Q_index must be an int or None, got str'), ], ids=[ 'negative index', - 'index too large', 'non-integer index', ], ) - def test_get_independent_variables_raises(self, diffusion_model, Q_index): + def test_get_independent_variables_raises( + self, diffusion_model, Q_index, expected_exception, expected_message + ): # WHEN THEN EXPECT - with pytest.raises(ValueError, match=r'Q_index must be an integer between 0 and'): + with pytest.raises(expected_exception, match=expected_message): diffusion_model.get_independent_variables(Q_index=Q_index) + def test_get_independent_variables_deferred_when_Q_is_none(self, diffusion_model): + # WHEN: Q is None, so the upper-bound check on Q_index is deferred + # THEN EXPECT: no exception, and no independent variables + assert diffusion_model.get_independent_variables(Q_index=100) == [] + @pytest.mark.parametrize( - 'Q_index', + ('Q_index', 'expected_exception', 'expected_message'), [ - -1, - 100, - 'string', + (-1, IndexError, 'Q_index must be non-negative'), + (100, IndexError, 'out of range'), + ('string', TypeError, 'Q_index must be an int or None, got str'), ], ids=[ 'negative index', @@ -193,9 +198,11 @@ def test_get_independent_variables_raises(self, diffusion_model, Q_index): 'non-integer index', ], ) - def test_get_all_variables_raises(self, diffusion_model, Q_index): + def test_get_all_variables_raises( + self, diffusion_model, Q_index, expected_exception, expected_message + ): # WHEN THEN EXPECT - with pytest.raises(ValueError, match=r'Q_index must be an integer between 0 and'): + with pytest.raises(expected_exception, match=expected_message): diffusion_model.get_all_variables(Q_index=Q_index) def test_create_component_collections_no_Q(self, diffusion_model): diff --git a/tests/unit/easydynamics/sample_model/test_model_base.py b/tests/unit/easydynamics/sample_model/test_model_base.py index 5b58370d6..56c0a9dbf 100644 --- a/tests/unit/easydynamics/sample_model/test_model_base.py +++ b/tests/unit/easydynamics/sample_model/test_model_base.py @@ -168,7 +168,7 @@ def test_get_all_variables_with_invalid_Q_index_raises(self, model_base): # WHEN / THEN / EXPECT with pytest.raises( IndexError, - match='Q_index 5 is out of bounds for component collections of length 3', + match='Q_index 5 is out of bounds for Q of length 3', ): model_base.get_all_variables(Q_index=5)