-
-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TypingHintArgSpecGuesser breaks on type alias List #216
Comments
Replaced ```python if args and (args[0] in cls.BASIC_TYPES): ``` with ```python if args and (args[0] in cls.BASIC_TYPES): ```
I went ahead and made a PR for this: #217 |
argh==0.31.0
breaks (my) argh.dispatch_command
call
Minimal example as discussed in the related PR:
|
@neithere : This still breaks for me with from typing import Optional, List
def func(x: Optional[List] = None):
return 'hello'
if __name__ == '__main__':
import argh
argh.dispatch_command(func) Note that replacing
|
Ahh, you're right. I fixed a more general case but it was not enough. Will ship the fix with 0.31.2 later today. BTW, we'll get rid of those aliases ( |
@neithere: I see you're active, so let me help you out with this. You might want to use this to make a more significant test: def issue_216_happens_annotations(func, annotation):
"""
Util to test what annotations make the
https://github.com/neithere/argh/issues/216
issue happen
"""
import argh
func.__annotations__['x'] = annotation
try:
argh.dispatch_command(func)
except IndexError as e:
if e.args[0] == 'tuple index out of range':
return True
except BaseException:
pass
return False
# Example:
from typing import Optional, List
from functools import partial
def func(x = None):
return 'hello'
there_is_an_issue = partial(issue_216_happens_annotations, func)
test_types = [List, Optional[list], Optional[List], Optional[List[int]]]
list(map(there_is_an_issue, test_types))
# [False, False, True, False] |
Combine this with this and you'll get some monster coverage: def type_combos(generic_types, type_variables):
"""
Generate "generic" using combinations of types such as
`Optional[List], Dict[Tuple, List], Callable[[List], Dict]`
from a list of generic types such as `Optional`, `List`, `Dict`, `Callable`
and a list of type variables that are used to parametrize these generic types.
:param generic_types: A list of generic types
:param type_variables: A list of type variables
:return: A generator that yields generic types
>>> from typing import Optional, Dict, Tuple, List, Callable
>>> list(type_combos([Optional, Tuple], [list, dict])) # doctest: +NORMALIZE_WHITESPACE
[typing.Optional[list], typing.Optional[dict],
typing.Tuple[list, dict], typing.Tuple[dict, list]]
More significant example:
>>> generic_types = [Optional, Callable, Dict, Tuple]
>>> type_variables = [tuple, dict, List]
>>>
>>> for combo in type_combos(generic_types, type_variables):
... print(combo)
...
typing.Optional[tuple]
typing.Optional[dict]
typing.Optional[typing.List]
typing.Callable[[typing.Tuple[dict, ...]], tuple]
typing.Callable[[typing.Tuple[typing.List, ...]], tuple]
typing.Callable[[typing.Tuple[dict, typing.List]], tuple]
typing.Callable[[typing.Tuple[typing.List, dict]], tuple]
typing.Callable[[typing.Tuple[tuple, ...]], dict]
typing.Callable[[typing.Tuple[typing.List, ...]], dict]
typing.Callable[[typing.Tuple[tuple, typing.List]], dict]
typing.Callable[[typing.Tuple[typing.List, tuple]], dict]
typing.Callable[[typing.Tuple[tuple, ...]], typing.List]
typing.Callable[[typing.Tuple[dict, ...]], typing.List]
typing.Callable[[typing.Tuple[tuple, dict]], typing.List]
typing.Callable[[typing.Tuple[dict, tuple]], typing.List]
typing.Dict[tuple, dict]
typing.Dict[tuple, typing.List]
typing.Dict[dict, tuple]
typing.Dict[dict, typing.List]
typing.Dict[typing.List, tuple]
typing.Dict[typing.List, dict]
typing.Tuple[tuple, dict, typing.List]
typing.Tuple[tuple, typing.List, dict]
typing.Tuple[dict, tuple, typing.List]
typing.Tuple[dict, typing.List, tuple]
typing.Tuple[typing.List, tuple, dict]
typing.Tuple[typing.List, dict, tuple]
"""
def generate_combos(generic_type, remaining_vars):
from itertools import permutations
from typing import Callable, Dict, Tuple
if generic_type is Callable:
# Separate one variable for the output type
for output_type in remaining_vars:
input_vars = [var for var in remaining_vars if var != output_type]
# Generate combinations of input types
for n in range(1, len(input_vars) + 1):
for input_combo in permutations(input_vars, n):
# Format single-element tuples correctly
if len(input_combo) == 1:
input_type = Tuple[input_combo[0], ...]
else:
input_type = Tuple[input_combo]
yield Callable[[input_type], output_type]
elif generic_type in [Dict, Tuple]:
required_params = 2 if generic_type is Dict else len(remaining_vars)
for combo in permutations(remaining_vars, required_params):
yield generic_type[combo]
else:
for type_var in remaining_vars:
yield generic_type[type_var]
for generic_type in generic_types:
yield from generate_combos(generic_type, type_variables) |
The good news is that, trying it out on my side, I find that, out of 109776 combinations tested, the only one that failed was our good old `Optional[List]'! I'll do a PR to add these tests. |
@thorwhalen oh wow, thanks, that's a lot going on there :) I'll need some time to read the code itself. I've enabled CI for that PR and it's failing with some strange error messages (and lots of them!). I don't have time to dig deep into that at the moment, so I think that for now I'll just rely on the results of your research and assume that the only known problematic combination is fixed, and therefore the fix can be shipped. That's a great result already. Later I'll need to think how to ensure optimal test coverage for the upcoming Argh 1.0 (it won't support type aliases like |
P.S.: FYI, I'm trying to comment on the latest developments in typing hints support in #107 :) |
Yes. Just wanted to see what a test covering a lot more combinations would look like. Just a proposal. Not essential. |
Sorry not to give further details but I had the choice between sending a quick issue or not mentioning it at all.
My script, that uses
argh
, stopped working when I updated toargh==0.31.0
fromargh==0.26.2
.The error
My guess is that the
len(args) == 0
case is not handled properly?Without looking further, I'd suggest:
instead?
If I have time, I'll look at it further this week and send a PR.
The text was updated successfully, but these errors were encountered: