fix: accept numpy integer types as Parachute trigger

rocketpy/rocket/parachute.py

@@ -1,4 +1,5 @@
 from inspect import Parameter, signature
+from numbers import Real
 
 import numpy as np
 
@@ -8,6 +9,27 @@
 from ..prints.parachute_prints import _ParachutePrints
 
 
+def _is_a_height_trigger(trigger):
+    """Whether ``trigger`` is a number this class will read as a height.
+
+    ``numbers.Real`` rather than ``(int, float)`` so that NumPy scalars are
+    accepted: ``numpy.float64`` happens to subclass ``float``, but
+    ``numpy.int64`` and ``numpy.float32`` subclass neither and were refused
+    even though every arithmetic use of them here works.
+
+    What that spelling leaves out is what should be left out. ``numpy.bool_``
+    and the complex types are not ``Real``, so they still fall through to the
+    error. ``bool`` is excluded by hand because it *is* an ``int``, and ``True``
+    would otherwise be taken as a height of one metre.
+
+    This is the single definition of the height form. ``StochasticParachute``
+    validates the same triggers before a ``Parachute`` is ever built and calls
+    this rather than restating it, because the two spellings drifted apart once
+    already.
+    """
+    return isinstance(trigger, Real) and not isinstance(trigger, bool)
+
+
 class Parachute:
     """Keeps information of the parachute, which is modeled as a hemispheroid.
 
@@ -363,7 +385,7 @@ def wrapper(p, h, y, sensors, u_dot):
             return
 
         # Numeric altitude trigger
-        if isinstance(trigger, (int, float)):
+        if _is_a_height_trigger(trigger):
             self._trigger_falling_only = True
 
             def triggerfunc(p, h, y, sensors, u_dot):  # pylint: disable=unused-argument

rocketpy/stochastic/stochastic_parachute.py

@@ -1,23 +1,26 @@
 """Defines the StochasticParachute class."""
 
 from rocketpy.rocket import Parachute
+from rocketpy.rocket.parachute import _is_a_height_trigger
 
 from .stochastic_model import StochasticModel, _sampler_seed
 
 
 def _is_a_trigger(member):
     """One of the three forms ``Parachute`` accepts, and no more.
 
-    ``(int, float)`` deliberately, matching ``Parachute``'s own check rather
-    than ``numbers.Real``: that would take ``numpy.int64``, which ``Parachute``
-    refuses, so widening here only moves the failure to create time. ``bool``
-    is excluded because it is an ``int``, and would arrive as a height of one.
+    The height form defers to ``Parachute``'s own predicate instead of
+    restating it. Both were written out separately before and drifted: this one
+    kept ``(int, float)`` while ``Parachute`` widened to ``numbers.Real``, so a
+    ``numpy.int64`` height was refused here even though the ``Parachute`` it
+    would have built accepts it. Calling the same function is what keeps the
+    promise that what this accepts is what a parachute accepts.
     """
     if callable(member):
         return True
     if isinstance(member, str):
         return member.lower() == "apogee"
-    return isinstance(member, (int, float)) and not isinstance(member, bool)
+    return _is_a_height_trigger(member)
 
 
 class StochasticParachute(StochasticModel):

tests/unit/rocket/test_parachute.py

@@ -130,3 +130,43 @@ def test_callable_trigger_arities_route_arguments(trigger, expects_udot):
     result = parachute.triggerfunc(800.0, 500.0, [0.0] * 6, [], [1.0] * 6)
     assert result is True
     assert parachute.triggerfunc._expects_udot is expects_udot
+
+
+@pytest.mark.parametrize(
+    "trigger",
+    [800, 800.0, np.int64(800), np.int32(800), np.float64(800), np.float32(800)],
+    ids=str,
+)
+def test_any_real_number_is_read_as_a_height(trigger):
+    """A height is anything ``numbers.Real``, not just ``int`` and ``float``.
+
+    The check used to be ``isinstance(trigger, (int, float))``. ``numpy.float64``
+    subclasses ``float`` and passed, but ``numpy.int64`` and ``numpy.float32``
+    subclass neither, so a height read out of a NumPy array raised even though
+    it compares and arithmetics exactly like the value that worked."""
+    parachute = _make_parachute(trigger=trigger)
+
+    # Truthiness rather than `is True`: comparing against a NumPy scalar gives
+    # back a numpy.bool_, which is not the `True` singleton.
+    # falling (vz < 0) and below the trigger height
+    assert parachute.triggerfunc(0.0, 700.0, [0.0] * 5 + [-1.0], [], None)
+    # falling but still above it
+    assert not parachute.triggerfunc(0.0, 900.0, [0.0] * 5 + [-1.0], [], None)
+    # below it but still ascending
+    assert not parachute.triggerfunc(0.0, 700.0, [0.0] * 5 + [1.0], [], None)
+
+
+@pytest.mark.parametrize(
+    "trigger",
+    [True, False, np.bool_(True), complex(800), np.complex64(800), "banana", None, {}],
+    ids=str,
+)
+def test_what_is_not_a_height_is_still_refused(trigger):
+    """Widening to ``numbers.Real`` must not turn the check into "anything".
+
+    ``bool`` is the one that has to be excluded by hand, because it *is* an
+    ``int``: ``True`` would otherwise be accepted and read as a height of one
+    metre, firing the parachute a metre above the ground. ``numpy.bool_`` and
+    the complex types need no special case, since neither is ``Real``."""
+    with pytest.raises(ValueError, match="Unable to set the trigger"):
+        _make_parachute(trigger=trigger)

tests/unit/stochastic/test_stochastic_parachute.py

@@ -5,6 +5,7 @@
 
 from rocketpy.stochastic import StochasticParachute
 from rocketpy.rocket.parachute import Parachute
+from rocketpy.stochastic.stochastic_parachute import _is_a_trigger
 
 
 def test_stochastic_parachute_create_object(stochastic_main_parachute):
@@ -79,35 +80,92 @@ def test_a_trigger_that_is_not_a_list_of_those_is_refused(calisto_main_chute, tr
 
 @pytest.mark.parametrize(
     "member",
-    [_at_apogee, "apogee", "APOGEE", 800, 800.0, np.float64(800)],
+    [
+        _at_apogee,
+        "apogee",
+        "APOGEE",
+        800,
+        800.0,
+        np.float64(800),
+        np.float32(800),
+        np.int64(800),
+        np.int32(800),
+    ],
     ids=str,
 )
 def test_what_this_accepts_is_what_a_parachute_accepts(calisto_main_chute, member):
     """The property, rather than a list of types. Anything this lets through
-    has to survive `Parachute`, or the check has only moved the failure."""
+    has to survive `Parachute`, or the check has only moved the failure.
+
+    The NumPy integers used to belong to the test below, refused by both
+    because `Parachute` spelled its height check `(int, float)`: `numpy.float64`
+    subclasses `float` and passed, `numpy.int64` subclasses neither and raised.
+    `Parachute` now reads a height as `numbers.Real`, so they are heights like
+    any other and belong here."""
     StochasticParachute(calisto_main_chute, trigger=[member])
 
     Parachute("probe", 10.0, member, 105, 1.5)
 
 
-@pytest.mark.parametrize("member", [np.int64(800), np.int32(800)], ids=str)
-def test_a_numpy_integer_is_refused_here_because_parachute_refuses_it(
-    calisto_main_chute, member
-):
-    """`Parachute` checks `isinstance(trigger, (int, float))`. `numpy.float64`
-    subclasses `float` and passes; `numpy.int64` subclasses neither and raises.
-
-    So this check matches that one rather than `numbers.Real`, which would be
-    the wider and more natural spelling but would let these through to fail at
-    create time. The asymmetry is `Parachute`'s and is worth fixing there.
-    """
+@pytest.mark.parametrize(
+    "member",
+    [np.bool_(True), complex(800), np.complex64(800), "banana", None, {}],
+    ids=str,
+)
+def test_what_this_refuses_is_what_a_parachute_refuses(calisto_main_chute, member):
+    """The other half of the same property, and the half that keeps the widened
+    height check honest.
+
+    `numbers.Real` was the wider spelling, but not an unbounded one: neither
+    `numpy.bool_` nor the complex types are `Real`, so they still reach the
+    error rather than being read as a height. `numpy.bool_` needs no exclusion
+    of its own for the same reason -- unlike `bool`, which is an `int` and is
+    ruled out by hand."""
     with pytest.raises(ValueError, match="Unable to set the trigger"):
         Parachute("probe", 10.0, member, 105, 1.5)
 
     with pytest.raises(AssertionError, match="must be a non-empty list"):
         StochasticParachute(calisto_main_chute, trigger=[member])
 
 
+def test_neither_check_can_drift_from_the_other_again():
+    """The two checks were written out separately and disagreed: a
+    `numpy.int64` height was refused in `stochastic/` and accepted by the
+    `Parachute` that would have been built from it. Nothing failed, because
+    each side had a test asserting its own half.
+
+    They now share one predicate, so this asserts the agreement itself over the
+    whole boundary rather than a list of types on either side."""
+    boundary = [
+        800,
+        800.0,
+        np.float64(800),
+        np.float32(800),
+        np.int64(800),
+        np.int32(800),
+        True,
+        np.bool_(True),
+        complex(800),
+        np.complex64(800),
+        "apogee",
+        "banana",
+        None,
+    ]
+
+    for member in boundary:
+        try:
+            Parachute("probe", 10.0, member, 105, 1.5)
+        except ValueError:
+            parachute_accepts = False
+        else:
+            parachute_accepts = True
+
+        assert _is_a_trigger(member) is parachute_accepts, (
+            f"{member!r}: stochastic/ says {_is_a_trigger(member)}, "
+            f"Parachute says {parachute_accepts}"
+        )
+
+
 def test_the_check_is_not_stripped_by_python_dash_o():
     """`python -O` removes an `assert` outright, and this check is the only
     thing between a bad trigger and a `Parachute` that either refuses it much