TypingHintArgSpecGuesser breaks on type alias List

[thorwhalen]

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 to argh==0.31.0 from argh==0.26.2.

The error

    argh.dispatch_command(populate_pkg_dir)
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/dispatching.py", line 470, in dispatch_command
    set_default_command(parser, function, name_mapping_policy=name_mapping_policy)
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 420, in set_default_command
    inferred_args: List[ParserAddArgumentSpec] = list(
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 183, in infer_argspecs_from_function
    TypingHintArgSpecGuesser.typing_hint_to_arg_spec_params(
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 778, in typing_hint_to_arg_spec_params
    item_type = cls._extract_item_type_from_list_type(first_subtype)
  File "~.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 799, in _extract_item_type_from_list_type
    if args[0] in cls.BASIC_TYPES:
IndexError: tuple index out of range

My guess is that the len(args) == 0 case is not handled properly?

Without looking further, I'd suggest:

 if args and (args[0] in cls.BASIC_TYPES):

instead?

If I have time, I'll look at it further this week and send a PR.

[thorwhalen]

I went ahead and made a PR for this: https://github.com/neithere/argh/pull/217

[neithere]

Minimal example as discussed in the related PR:

• def func(xs: list) — works
• def func(xs: List) — breaks

[thorwhalen]

@neithere : This still breaks for me with argh==0.31.1:

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 Optional[List] with Optional[list] or just List works though.

  File "~/tmp/scrap.py", line 9, in <module>
    argh.dispatch_command(func)
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/dispatching.py", line 470, in dispatch_command
    set_default_command(parser, function, name_mapping_policy=name_mapping_policy)
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 420, in set_default_command
    inferred_args: List[ParserAddArgumentSpec] = list(
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 183, in infer_argspecs_from_function
    TypingHintArgSpecGuesser.typing_hint_to_arg_spec_params(
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 778, in typing_hint_to_arg_spec_params
    item_type = cls._extract_item_type_from_list_type(first_subtype)
  File "~/.pyenv/versions/p10/lib/python3.10/site-packages/argh/assembling.py", line 799, in _extract_item_type_from_list_type
    if args[0] in cls.BASIC_TYPES:
IndexError: tuple index out of range

[neithere]

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 (List etc.) when we drop support for Python 3.8 (pretty soon), they are deprecated since Python 3.9, so I'd really try to use Optional[list] or even list | None whenever possible.

[thorwhalen]

@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]

[thorwhalen]

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)

[thorwhalen]

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]

@neithere -- made a PR containing my code to increase the test coverage for this problem. Uses (further edited) versions of my code I pasted above, plus a test function to apply it to argh.dispatch over a large (100K+) number of type combinations.

https://github.com/neithere/argh/pull/219

[neithere]

@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 List, BTW). At the moment I'm leaning towards writing tests specifically for what Argh knows how to interpret, and so far it's a pretty limited set of types. It also won't go too deep inside of those because of CLI limitations (e.g. pythonic foo: list[str] is foo, [foo ...] in the CLI world, but what could foo: list[list[str]] mean in CLI? how would you express a dict in a set of CLI arguments?). Of course we also need to ensure that the library doesn't crash on whatever it does not support, so a generator of permutations might be useful indeed, and I guess that's exactly the problem that your PR addresses.

[neithere]

P.S.: FYI, I'm trying to comment on the latest developments in typing hints support in #107 :)

[thorwhalen]

@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 List, BTW). At the moment I'm leaning towards writing tests specifically for what Argh knows how to interpret, and so far it's a pretty limited set of types. It also won't go too deep inside of those because of CLI limitations (e.g. pythonic foo: list[str] is foo, [foo ...] in the CLI world, but what could foo: list[list[str]] mean in CLI? how would you express a dict in a set of CLI arguments?). Of course we also need to ensure that the library doesn't crash on whatever it does not support, so a generator of permutations might be useful indeed, and I guess that's exactly the problem that your PR addresses.

Yes. Just wanted to see what a test covering a lot more combinations would look like. Just a proposal. Not essential.