py_trees
py_trees copied to clipboard
Selector __init__ is incorrectly typed
Selector's init is currently (2.1.6) typed as:
def __init__(self, name="Selector", memory=False, children=None):
but should match Sequence and Parallel:
def __init__(
self,
name: str="Selector",
memory: bool=True,
children: typing.List[behaviour.Behaviour]=None
):
in the current state, you can to something like this:
py_trees.composites.Selector('MySelector', [... behaviours here])
and mypy will allow it, but the list I guess gets interpreted as the boolean and the Selector has no children.
Explicitly setting memory, or using children=
as a named variable fixes the problem.
seems my initial diagnosis is wrong and this in an inherent problem with using boolean positional arguments in python. It looks like this may be a solution once it lands:
https://github.com/python/mypy/pull/10557
Aside from the boolean thing, these are examples of cases mypy missed when I ran it on the package (mypy -p py_trees
). Not sure I understand why mypy missed them.
How are you catching these? The only thing I can guess at is perhaps because you're running it on the tests as well?
Also - should fix the typing for children regardless. It should be typing.Optional[typing.List[behaviour.Behaviour]]
.
How are you catching these? The only thing I can guess at is perhaps because you're running it on the tests as well?
two reasons: first, see the https://github.com/splintered-reality/py_trees/pull/338 PR, where I enabled mypy to run on tests as well. Second, when upgrading our own project to use the new release, a lot of things broke, and this was one of the tricky ones.
these are examples of cases mypy missed when I ran it on the package (
mypy -p py_trees
). Not sure I understand why mypy missed them.
its because mypy is a bit "soft" by default. I have a second branch I'll create a PR for in just a minute that will enable additional checks. The additional checks found many issues, which I also fixed.
children: typing.Optional[typing.List[behaviour.Behaviour]] = None
Fixed in #380.