Issue with Callable type for < Python 3.9

[sagarbadiyani]

This issue is reproducible in Python 3.8 but not reproducible in Python 3.10

Consider a file some/module.py

class Foo:
    def __init__(self):
        self.some_dict = {
            "foo": self.bar
        }
    
    def bar(self):
        pass
        
    def zzz(self, a, b):
        pass

Consider another file myscript.py

from some.module import Foo

foo = Foo()
foo.zzz("something", foo.some_dict)

Run monkeytype run myscript.py
Then run monkeytype stub some.module

This throws an error

  File "<redacted>/monkeytype/encoding.py", line 203, in to_trace
    arg_types = arg_types_from_json(self.arg_types)
  File "<redacted>/monkeytype/encoding.py", line 151, in arg_types_from_json
    return {name: type_from_dict(type_dict) for name, type_dict in arg_types.items()}
  File "<redacted>/monkeytype/encoding.py", line 151, in <dictcomp>
    return {name: type_from_dict(type_dict) for name, type_dict in arg_types.items()}
  File "<redacted>/monkeytype/encoding.py", line 122, in type_from_dict
    elem_types = tuple(type_from_dict(e) for e in elem_type_dicts)
  File "<redacted>/monkeytype/encoding.py", line 122, in <genexpr>
    elem_types = tuple(type_from_dict(e) for e in elem_type_dicts)
  File "<redacted>/monkeytype/encoding.py", line 126, in type_from_dict
    typ = typ[elem_types]  # type: ignore[index]
  File "/usr/local/lib/python3.8/typing.py", line 806, in __getitem__
    raise TypeError("Callable must be used as "
TypeError: Callable must be used as Callable[[arg, ...], result].

As we can see, the input type of b in zzz is Dict[str, Callable]
On trying to serialize the trace, I found that in Python 3.8, the arg type for b was

{"elem_types": [{"module": "builtins", "qualname": "str"}, {"elem_types": [], "module": "typing", "qualname": "Callable"}], "module": "typing", "qualname": "Dict"}

While in 3.10, the arg type for b was

{"elem_types": [{"module": "builtins", "qualname": "str"}, {"module": "typing", "qualname": "Callable"}], "module": "typing", "qualname": "Dict"}

Notice an extra "elem_types": [], in Py38

Upon digging further I found that this is because type_to_dict(typing.Callable) returns
{'module': 'typing', 'qualname': 'Callable'} in Py310,
but {"elem_types": [], 'module': 'typing', 'qualname': 'Callable'} in Py38

Should we consider adding an explicit check for elem_types being [] at https://github.com/carljm/MonkeyType/blob/ff7cc071971cc33e1b3e29df1fbbef85c7fbf89a/monkeytype/encoding.py#L83 ?
(d["elem_types"] = [type_to_dict(t) for t in elem_types])

If yes, I would be happy to raise a PR for this.

Suggested change

- d["elem_types"] = [type_to_dict(t) for t in elem_types]
+ if typ != typing.Callable or elem_types:
+     d["elem_types"] = [type_to_dict(t) for t in elem_types]

But I am not confident of this suggested change since it might cause regression given elem_types has been explicitly set to () just 1 line above, in which case, should Callable be handled explicitly?

Or should this be handled while stubbing in encoding.py in def type_from_dict() at

    if elem_type_dicts is not None and is_generic(typ):
        elem_types = tuple(type_from_dict(e) for e in elem_type_dicts)

[sagarbadiyani]

@carljm can you please guide me here on how to proceed? Please let me know if you require more information on this, I would be happy to work on this

[carljm]

Hi @sagarbadiyani -- unfortunately it may be up to a few weeks before I have time to dig into this issue and recommend what I think is the best approach; it looks like it's a bit subtle. If you are able to reach a solution sooner that you are confident is the correct one, I can try to find time to look at a PR.

[sagarbadiyani]

Sure, @carljm, I will try to solutionize this.
Thank you.

[cam-laf]

@sagarbadiyani I've encountered the same issue on Python 3.7.3.
The fix that worked for me was:

https://github.com/carljm/MonkeyType/blob/ff7cc071971cc33e1b3e29df1fbbef85c7fbf89a/monkeytype/encoding.py#L121

-   if elem_type_dicts is not None and is_generic(typ):
+     if elem_type_dicts and is_generic(typ):
        elem_types = tuple(type_from_dict(e) for e in elem_type_dicts)
        # mypy complains that a value of type `type` isn't indexable. That's
        # true, but we know typ is a subtype that is indexable. Even checking
        # with hasattr(typ, '__getitem__') doesn't help
+     if elem_types:
-      typ = typ[elem_types]  # type: ignore[index]                        
+        typ = typ[elem_types]  # type: ignore[index]