diff --git a/rocketpy/stochastic/stochastic_aero_surfaces.py b/rocketpy/stochastic/stochastic_aero_surfaces.py index 07c50f8ee..27d3d89a9 100644 --- a/rocketpy/stochastic/stochastic_aero_surfaces.py +++ b/rocketpy/stochastic/stochastic_aero_surfaces.py @@ -94,9 +94,11 @@ def _validate_kind(self, kind): must be a list of strings.""" if kind is not None: # TODO: Never vary the kind of the nose cone. It is a fixed parameter. - assert isinstance(kind, list) and all( - isinstance(member, str) for member in kind - ), "`kind` must be a list of strings" + if not ( + isinstance(kind, list) + and all(isinstance(member, str) for member in kind) + ): + raise AssertionError("`kind` must be a list of strings") def create_object(self): """Creates and returns a NoseCone object from the randomly generated diff --git a/rocketpy/stochastic/stochastic_environment.py b/rocketpy/stochastic/stochastic_environment.py index e0fc33eec..95845f51f 100644 --- a/rocketpy/stochastic/stochastic_environment.py +++ b/rocketpy/stochastic/stochastic_environment.py @@ -134,19 +134,27 @@ def _validate_ensemble(self, ensemble_member, environment): return if ensemble_member is not None: - assert isinstance(ensemble_member, list), "`ensemble_member` must be a list" - assert all( - isinstance(member, int) and member >= 0 for member in ensemble_member - ), "`ensemble_member` must be a list of positive integers" - assert ( + if not isinstance(ensemble_member, list): + raise AssertionError("`ensemble_member` must be a list") + if not ( + all( + isinstance(member, int) and member >= 0 + for member in ensemble_member + ) + ): + raise AssertionError( + "`ensemble_member` must be a list of positive integers" + ) + if not ( 0 <= min(ensemble_member) <= max(ensemble_member) < environment.num_ensemble_members - ), ( - "`ensemble_member` must be in the range from 0 to " - + f"{environment.num_ensemble_members - 1}" - ) + ): + raise AssertionError( + "`ensemble_member` must be in the range from 0 to " + + f"{environment.num_ensemble_members - 1}" + ) setattr(self, "ensemble_member", ensemble_member) else: # if no ensemble member is provided, get it from the environment diff --git a/rocketpy/stochastic/stochastic_flight.py b/rocketpy/stochastic/stochastic_flight.py index ecac053a3..525526798 100644 --- a/rocketpy/stochastic/stochastic_flight.py +++ b/rocketpy/stochastic/stochastic_flight.py @@ -79,9 +79,8 @@ def __init__( reaches this time, it will terminate. This attribute can not be randomized. """ if terminate_on_apogee is not None: - assert isinstance(terminate_on_apogee, bool), ( - "`terminate_on_apogee` must be a boolean" - ) + if not isinstance(terminate_on_apogee, bool): + raise AssertionError("`terminate_on_apogee` must be a boolean") if time_overshoot is not None: if not isinstance(time_overshoot, bool): raise TypeError("`time_overshoot` must be a boolean") @@ -109,15 +108,17 @@ def __init__( def _validate_initial_solution(self, initial_solution): if initial_solution is not None: if isinstance(initial_solution, (tuple, list)): - assert len(initial_solution) == 14, ( - "`initial_solution` must be a 14 element tuple, the " - "elements are:\n t_initial, x_init, y_init, z_init, " - "vx_init, vy_init, vz_init, e0_init, e1_init, e2_init, " - "e3_init, w1Init, w2Init, w3Init" - ) - assert all(isinstance(i, (int, float)) for i in initial_solution), ( - "`initial_solution` must be a tuple of numbers" - ) + if not len(initial_solution) == 14: + raise AssertionError( + "`initial_solution` must be a 14 element tuple, the " + "elements are:\n t_initial, x_init, y_init, z_init, " + "vx_init, vy_init, vz_init, e0_init, e1_init, e2_init, " + "e3_init, w1Init, w2Init, w3Init" + ) + if not all(isinstance(i, (int, float)) for i in initial_solution): + raise AssertionError( + "`initial_solution` must be a tuple of numbers" + ) else: raise TypeError("`initial_solution` must be a tuple of numbers") diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 5bb0598bf..be2438a0c 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -177,13 +177,15 @@ def _validate_tuple(self, input_name, input_value, getattr=getattr): # pylint: AssertionError If the input is not in a valid format. """ - assert len(input_value) in [ + if len(input_value) not in [ 2, 3, - ], f"'{input_name}': tuple must have length 2 or 3" - assert isinstance(input_value[0], (int, float)), ( - f"'{input_name}': First item of tuple must be an int or float" - ) + ]: + raise AssertionError(f"'{input_name}': tuple must have length 2 or 3") + if not isinstance(input_value[0], (int, float)): + raise AssertionError( + f"'{input_name}': First item of tuple must be an int or float" + ) if len(input_value) == 2: return self._validate_tuple_length_two(input_name, input_value, getattr) @@ -214,9 +216,10 @@ def _validate_tuple_length_two(self, input_name, input_value, getattr=getattr): AssertionError If the input is not in a valid format. """ - assert isinstance(input_value[1], (int, float, str)), ( - f"'{input_name}': second item of tuple must be an int, float, or string." - ) + if not isinstance(input_value[1], (int, float, str)): + raise AssertionError( + f"'{input_name}': second item of tuple must be an int, float, or string." + ) if isinstance(input_value[1], str): # if second item is a string, then it is assumed that the first item @@ -260,14 +263,16 @@ def _validate_tuple_length_three(self, input_name, input_value, getattr=getattr) AssertionError If the input is not in a valid format. """ - assert isinstance(input_value[1], (int, float)), ( - f"'{input_name}': Second item of a tuple with length 3 must be an " - "int or float." - ) - assert isinstance(input_value[2], str), ( - f"'{input_name}': Third item of tuple must be a string containing the " - "name of a valid numpy.random distribution function." - ) + if not isinstance(input_value[1], (int, float)): + raise AssertionError( + f"'{input_name}': Second item of a tuple with length 3 must be an " + "int or float." + ) + if not isinstance(input_value[2], str): + raise AssertionError( + f"'{input_name}': Third item of tuple must be a string containing the " + "name of a valid numpy.random distribution function." + ) dist_func = get_distribution(input_value[2], self.__random_number_generator) return (input_value[0], input_value[1], dist_func) @@ -382,14 +387,18 @@ def _validate_tuple_factor(self, input_name, factor_tuple): AssertionError If the input is not in a valid format. """ - assert len(factor_tuple) in [ + if len(factor_tuple) not in [ 2, 3, - ], f"'{input_name}`: Factors tuple must have length 2 or 3" - assert all(isinstance(item, (int, float)) for item in factor_tuple[:2]), ( - f"'{input_name}`: First and second items of Factors tuple must be " - "either an int or float" - ) + ]: + raise AssertionError( + f"'{input_name}`: Factors tuple must have length 2 or 3" + ) + if not all(isinstance(item, (int, float)) for item in factor_tuple[:2]): + raise AssertionError( + f"'{input_name}`: First and second items of Factors tuple must be " + "either an int or float" + ) if len(factor_tuple) == 2: return ( @@ -398,10 +407,11 @@ def _validate_tuple_factor(self, input_name, factor_tuple): get_distribution("normal", self.__random_number_generator), ) elif len(factor_tuple) == 3: - assert isinstance(factor_tuple[2], str), ( - f"'{input_name}`: Third item of tuple must be a string containing " - "the name of a valid numpy.random distribution function" - ) + if not isinstance(factor_tuple[2], str): + raise AssertionError( + f"'{input_name}`: Third item of tuple must be a string containing " + "the name of a valid numpy.random distribution function" + ) dist_func = get_distribution( factor_tuple[2], self.__random_number_generator ) @@ -428,9 +438,10 @@ def _validate_list_factor(self, input_name, factor_list): AssertionError If the input is not in a valid format. """ - assert all(isinstance(item, (int, float)) for item in factor_list), ( - f"'{input_name}`: Items in list must be either ints or floats" - ) + if not all(isinstance(item, (int, float)) for item in factor_list): + raise AssertionError( + f"'{input_name}`: Items in list must be either ints or floats" + ) return factor_list def _validate_1d_array_like(self, input_name, input_value): @@ -482,9 +493,15 @@ def _validate_positive_int_list(self, input_name, input_value): If the input is not in a valid format. """ if input_value is not None: - assert isinstance(input_value, list) and all( - isinstance(member, int) and member >= 0 for member in input_value - ), f"`{input_name}` must be a list of positive integers" + if not ( + isinstance(input_value, list) + and all( + isinstance(member, int) and member >= 0 for member in input_value + ) + ): + raise AssertionError( + f"`{input_name}` must be a list of positive integers" + ) def _reset_custom_samplers(self, seed): """Give each sampler its own stream, and each shared group one between @@ -570,14 +587,18 @@ def _validate_airfoil(self, airfoil): """ # TODO: The _validate_airfoil should be defined in a child class. if airfoil is not None: - assert isinstance(airfoil, list) and all( - isinstance(member, tuple) for member in airfoil - ), "`airfoil` must be a list of tuples" + if not ( + isinstance(airfoil, list) + and all(isinstance(member, tuple) for member in airfoil) + ): + raise AssertionError("`airfoil` must be a list of tuples") for member in airfoil: - assert len(member) == 2, "`airfoil` tuples must have length 2" - assert isinstance(member[1], str), ( - "`airfoil` tuples must have a string as the second item" - ) + if not len(member) == 2: + raise AssertionError("`airfoil` tuples must have length 2") + if not isinstance(member[1], str): + raise AssertionError( + "`airfoil` tuples must have a string as the second item" + ) if isinstance(member[0], list): if len(np.shape(member[0])) != 2 and np.shape(member[0])[1] != 2: raise AssertionError("`airfoil` tuples must have shape (n,2)") diff --git a/rocketpy/stochastic/stochastic_parachute.py b/rocketpy/stochastic/stochastic_parachute.py index 787a98cd7..19ab3dab0 100644 --- a/rocketpy/stochastic/stochastic_parachute.py +++ b/rocketpy/stochastic/stochastic_parachute.py @@ -156,12 +156,14 @@ def _validate_noise(self, noise): (mean, standard deviation, time-correlation) """ if noise is not None: - assert isinstance(noise, list) and all( - isinstance(member, tuple) for member in noise - ), ( - "`noise` must be a list of tuples in the form of " - "(mean, standard deviation, time-correlation)" - ) + if not ( + isinstance(noise, list) + and all(isinstance(member, tuple) for member in noise) + ): + raise AssertionError( + "`noise` must be a list of tuples in the form of " + "(mean, standard deviation, time-correlation)" + ) def create_object(self): """Creates and returns a Parachute object from the randomly generated diff --git a/tests/unit/stochastic/test_custom_sampler.py b/tests/unit/stochastic/test_custom_sampler.py index ae3d906ba..286d97cd3 100644 --- a/tests/unit/stochastic/test_custom_sampler.py +++ b/tests/unit/stochastic/test_custom_sampler.py @@ -1,3 +1,5 @@ +import subprocess +import sys from types import SimpleNamespace import numpy as np @@ -361,9 +363,6 @@ def test_a_non_sampler_is_refused_even_under_optimisation(): def test_the_refusal_survives_python_dash_o(): """The mechanism, not just the behaviour: run it in a child with -O and check the exception still arrives.""" - import subprocess - import sys - program = ( "from types import SimpleNamespace;" "from rocketpy.stochastic.stochastic_model import StochasticModel;" diff --git a/tests/unit/stochastic/test_validation_under_optimisation.py b/tests/unit/stochastic/test_validation_under_optimisation.py new file mode 100644 index 000000000..2bf4ca535 --- /dev/null +++ b/tests/unit/stochastic/test_validation_under_optimisation.py @@ -0,0 +1,71 @@ +"""Validation in `stochastic/` has to survive `python -O`. + +`assert` is removed by the optimiser, so every check written that way stops +running under `-O` and malformed user input reaches the model instead. The +module carried a TODO asking for this; these tests are what keeps it done. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[3] + +MODULES = [ + "stochastic_model", + "stochastic_environment", + "stochastic_flight", + "stochastic_aero_surfaces", + "stochastic_parachute", +] + + +@pytest.mark.parametrize("module", MODULES) +def test_no_production_asserts_remain(module): + """The mechanism. A single `assert` reintroduced here is a check that stops + existing under `-O`, which is exactly what this file exists to prevent.""" + source = (REPO / "rocketpy" / "stochastic" / f"{module}.py").read_text() + offenders = [ + f"{n}: {line.strip()}" + for n, line in enumerate(source.splitlines(), 1) + if line.strip().startswith("assert ") + ] + + assert not offenders, f"{module}.py still asserts: {offenders}" + + +REFUSALS = [ + ("a tuple element of the wrong type", "mass=('not a number', 0.5)"), + ("a tuple of the wrong length", "mass=(1.0, 0.5, 'normal', 'extra')"), +] + + +@pytest.mark.parametrize("label, kwargs", REFUSALS, ids=[r[0] for r in REFUSALS]) +def test_malformed_input_is_refused_under_optimisation(label, kwargs): + """The behaviour, in a child interpreter under -O. + + Run in-process this passes either way, because the assert is still compiled + in. The optimiser only strips it at compile time, so the check has to be a + separate interpreter to mean anything. + """ + program = ( + "from types import SimpleNamespace;" + "from rocketpy.stochastic.stochastic_model import StochasticModel;" + "obj = SimpleNamespace(mass=1.0)\n" + "try:\n" + f" StochasticModel(obj, {kwargs})\n" + " print('accepted')\n" + "except AssertionError:\n" + " print('refused')\n" + ) + done = subprocess.run( + [sys.executable, "-O", "-c", program], + capture_output=True, + text=True, + check=True, + cwd=REPO, + ) + + assert "refused" in done.stdout, f"{label}: {done.stdout!r} {done.stderr!r}"