Describe the bug
StochasticParachute._validate_trigger rejects the callable triggers its own docstring says it accepts:
assert isinstance(trigger, list) and all(
isinstance(member, (str, int, float) or callable(member))
for member in trigger
), "`trigger` must be a list of callables, string 'apogee' or ints/floats"
The or sits inside the isinstance call rather than beside it. (str, int, float) is a non-empty tuple and therefore truthy, so X or callable(member) short-circuits to X and callable(member) is never evaluated. The check reduces to isinstance(member, (str, int, float)), and a callable is none of those.
Parachute accepts a callable trigger, Flight calls it, and the docstring three lines above says "a list of callables, string 'apogee' or ints/floats". Only the stochastic wrapper refuses one.
To Reproduce
def at_apogee(pressure, height, state):
return state[5] < 0
base = Parachute("c", 10.0, "apogee", 105, 1.5)
StochasticParachute(base, trigger=[at_apogee])
[callable] REJECTED
["apogee"] accepted
[800] accepted
[callable, 800] REJECTED
The two documented non-callable forms pass, so the argument reaches the check and the check is what refuses it.
Expected behavior
A list of callables is accepted, matching the docstring and Parachute. Moving the or out of the call is the whole fix:
assert isinstance(trigger, list) and all(
isinstance(member, (str, int, float)) or callable(member)
for member in trigger
), "`trigger` must be a list of callables, string 'apogee' or ints/floats"
Worth a test with a callable in the list, since the current expression passes every test that only uses "apogee" or a number.
Additional context
Not a regression, and not touched by #1054. Found while reviewing that PR.
Related: #670 (migrate parachute triggers into controllers), #1086 (triggers evaluated twice per time node).
Verified on develop at 1691119, Python 3.12.13.
Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com
Describe the bug
StochasticParachute._validate_triggerrejects the callable triggers its own docstring says it accepts:The
orsits inside theisinstancecall rather than beside it.(str, int, float)is a non-empty tuple and therefore truthy, soX or callable(member)short-circuits toXandcallable(member)is never evaluated. The check reduces toisinstance(member, (str, int, float)), and a callable is none of those.Parachuteaccepts a callable trigger,Flightcalls it, and the docstring three lines above says "a list of callables, string 'apogee' or ints/floats". Only the stochastic wrapper refuses one.To Reproduce
The two documented non-callable forms pass, so the argument reaches the check and the check is what refuses it.
Expected behavior
A list of callables is accepted, matching the docstring and
Parachute. Moving theorout of the call is the whole fix:Worth a test with a callable in the list, since the current expression passes every test that only uses
"apogee"or a number.Additional context
Not a regression, and not touched by #1054. Found while reviewing that PR.
Related: #670 (migrate parachute triggers into controllers), #1086 (triggers evaluated twice per time node).
Verified on
developat1691119, Python 3.12.13.Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com