Skip to content

Commit bd9317b

Browse files
committed
A boolean is not a rate, and an oversized int is not a different error
YAML reads `yes` and `on` as True and `float(True)` is 1.0, so `sampling_rate: yes` became 1 Hz instead of 100 with nothing said. The previous revision of the test file pinned that as the contract, which is worse than leaving it unspecified. Refused now, the same way a boolean is already refused as a range endpoint. `float(10**400)` raises OverflowError, so one argument in the same validator came back as a different exception type from everything beside it. 126 tests, up from 122. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 2fa75e6 commit bd9317b

2 files changed

Lines changed: 30 additions & 4 deletions

File tree

rocketpy/rocket/actuator/actuator.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,16 @@ def _finite_or_raise(value, description):
1717
refused, since an actuator cannot start at or be driven to one, and a value
1818
that is not a number at all is refused here rather than a few lines later
1919
inside ``np.clip``.
20+
21+
``bool`` is refused with them. YAML reads ``yes`` and ``on`` as ``True``, and
22+
``float(True)`` is 1.0, so a sampling rate written that way became 1 Hz with
23+
nothing said.
2024
"""
25+
if isinstance(value, bool):
26+
raise ValueError(f"{description} must be a number, not a boolean.")
2127
try:
2228
number = float(value)
23-
except (TypeError, ValueError) as error:
29+
except (TypeError, ValueError, OverflowError) as error:
2430
raise ValueError(f"{description} must be a number.") from error
2531
if not np.isfinite(number):
2632
raise ValueError(f"{description} must be a finite number.")

tests/unit/rocket/test_actuators.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -797,12 +797,10 @@ def _rate_of(rocket, attribute, per_axis):
797797
return (actuator.x if per_axis else actuator).demand_rate
798798

799799
@pytest.mark.parametrize("adder, attribute, extra, per_axis", ADDERS)
800-
@pytest.mark.parametrize("given", ["100", True, 100])
800+
@pytest.mark.parametrize("given", ["100", 100, 100.0])
801801
def test_both_halves_hold_the_same_normalized_value(
802802
self, calisto, adder, attribute, extra, per_axis, given
803803
):
804-
"""``True`` is in here because ``float(True)`` is 1.0, so a bool reaches
805-
the actuator as a rate and used to reach the controller as a bool."""
806804
getattr(calisto, adder)(
807805
controller_function=_no_op_controller, sampling_rate=given, **extra
808806
)
@@ -866,3 +864,25 @@ def test_an_accepted_second_call_still_replaces(self, calisto, adder, extra):
866864

867865
assert len(calisto._controllers) == 1
868866
assert calisto._controllers[0].sampling_rate == 50.0
867+
868+
869+
class TestABooleanIsNotARate:
870+
"""YAML reads `yes` and `on` as True, and float(True) is 1.0.
871+
872+
A sampling rate written that way became 1 Hz with nothing said, and an
873+
earlier revision of this file pinned that as the contract.
874+
"""
875+
876+
@pytest.mark.parametrize(
877+
"option",
878+
["demand_rate", "throttle_rate_limit", "throttle_time_constant"],
879+
)
880+
def test_a_boolean_is_refused(self, option):
881+
with pytest.raises(ValueError, match="boolean"):
882+
ThrottleActuator(**{option: True})
883+
884+
def test_an_integer_too_large_for_a_float_is_a_value_error(self):
885+
"""`float(10**400)` raises OverflowError, so the validator leaked a
886+
different exception type than everything beside it."""
887+
with pytest.raises(ValueError):
888+
ThrottleActuator(demand_rate=10**400)

0 commit comments

Comments
 (0)