Annotate braintree's MultipleValueNodeBuilder.in_list method - #16235
Conversation
This comment has been minimized.
This comment has been minimized.
donbarbos
left a comment
donbarbos
left a comment
There was a problem hiding this comment.
Thank you! Just one nit
| whitelist: Incomplete | ||
| def __init__(self, name, whitelist=[]) -> None: ... | ||
| def in_list(self, *values): ... | ||
| def in_list(self, *values: str) -> Search.Node: ... |
There was a problem hiding this comment.
Since a list can be used as such a sequence of values for the first argument, I think it might be worth adding an overload for this case.
(Please, import Unused from _typeshed)
| def in_list(self, *values: str) -> Search.Node: ... | |
| @overload | |
| def in_list(self, value: list[str], *values: Unused) -> Search.Node: ... | |
| @overload | |
| def in_list(self, *values: str) -> Search.Node: ... | |
There was a problem hiding this comment.
I'm not familiar with braintree, but I assume calling in_list if a list as first argument and further arguments would indicate a bug or misunderstanding of the API. In this case I think it's safer to remove the *values argument from the first overload.
There was a problem hiding this comment.
Yes, if the first argument is a list, it uses it and ignores the rest:
def in_list(self, *values):
if isinstance(values[0], list):
values = values[0]Passing a list and anything following it would not throw an error, but also would not make any sense. I removed the *values: Unused bit from the PR.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
3 participants
This adds type annotations for
braintree.search.Search.MultipleValueNodeBuilder.in_list.Looking at the source, the return value can only be Search.Node.
It's less clear what the type of
*valueselements can be, but grepping around the source it looked like it's onlystr.