You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm not strongly-versed in python typing (especially in pre-3.9 versions where hints became much more intuitive), otherwise I would have sent a PR directly, but trying to understand.
Call's __init__ method has a argument returns (and deocde_output()), which is defined as such
Since the second argument to the list can be None, would it be better for the Callable to be replaced with Optional[Callable] or simply Callable | None (or I guess in <3.9 syntax, Union[Callable, None])?
I have never seen the use of Tuples anywhere - both the examples and the tests use lists - but changing Tuple to List throws its own error since it's not expecting more than one argument to the List type. Changing it to Iterable[str, Optional[Callable]], while passing lint check, seems wonky because it definitely doesn't accept dicts.
What's the best way to properly represent the inputs?
This is relatively minor in the grand scope of things, but is an annoyance during development, and being technically correct is the best kind of correct.
The text was updated successfully, but these errors were encountered:
"but changing Tuple to List throws its own error since it's not expecting more than one argument to the List type."
This is the reason it says Tuple instead of List, but for your typechecker it should be good enough
"Since the second argument to the list can be None, would it be better for the Callable to be replaced with Optional[Callable]"
This would be fine
"while passing lint check, seems wonky because it definitely doesn't accept dicts."
I guess it might accept a dict if dict.keys() are the args, but that's weird :P Iterable seems most technically correct to me, I wouldn't be opposed to using that.
I'm not strongly-versed in python typing (especially in pre-3.9 versions where hints became much more intuitive), otherwise I would have sent a PR directly, but trying to understand.
Call
's__init__
method has a argumentreturns
(anddeocde_output()
), which is defined as suchSince the second argument to the list can be
None
, would it be better for theCallable
to be replaced withOptional[Callable]
or simplyCallable | None
(or I guess in <3.9 syntax,Union[Callable, None]
)?I have never seen the use of Tuples anywhere - both the examples and the tests use lists - but changing
Tuple
toList
throws its own error since it's not expecting more than one argument to the List type. Changing it toIterable[str, Optional[Callable]]
, while passing lint check, seems wonky because it definitely doesn't accept dicts.What's the best way to properly represent the inputs?
This is relatively minor in the grand scope of things, but is an annoyance during development, and being technically correct is the best kind of correct.
The text was updated successfully, but these errors were encountered: