dropbox / pyannotate

Auto-generate PEP-484 annotations
Apache License 2.0
1.42k stars 59 forks source link

Treat any instance of a `Mapping` as a `Dict`. #45

Closed rowillia closed 6 years ago

rowillia commented 6 years ago

Without this we're winding up with a bunch types like:

Union[Dict[str, str], OrderedDict]

On functions that can accept any Mapping.

I was woried about covariance/contravariance with this and mypy seems to be OK with that:

from collections import OrderedDict
from typing import Dict

def foo(a: Dict[str, str]) -> Dict[str, str]:
    return OrderedDict([('a', 'a'), ('b', 'b')])

foo(OrderedDict([('a', 'a'), ('b', 'b')]))
gvanrossum commented 6 years ago

Can you change this translation to the second pass (i.e. pyannotate_tools/annotations)? The isinstance(..., collections.Mapping) check worries me (it could be slow, and there was a bug here where typing_extensions caused endless recursion in such a check -- #36).

rowillia commented 6 years ago

@gvanrossum Thanks for the feedback! Instead of using isinstance I'm specifically checking for the most common ones I found (defaultdict and OrderedDict) now. There's a test case here to verify there's no infinite recursion, anything else I could/should test?

gvanrossum commented 6 years ago

I'm still not happy with doing any kind of unification of this kind during runtime collection. There are important API differences between Dict and OrderedDict and I don't want those to be lost in type_info.json. You haven't explained to me why you can't do this in the second pass.

rowillia commented 6 years ago

@gvanrossum The reason this is happening in the first pass is if we do this in the second pass we don't collect any information on key/value types. We can add a DefaultDict and OrderedDict type to type collection as well and then unify those in the second pass if you would prefer.

gvanrossum commented 6 years ago

Oh! But I would like to still keep the actual type (dict, collections.defaultdict or collections.OrderedDict) in type_info.json. If you can do that I think this is a great improvement!

gvanrossum commented 6 years ago

Do you think you will have time for this soon? I am waiting to release version 1.0.3.

rowillia commented 6 years ago

@gvanrossum Working on it now!

gvanrossum commented 6 years ago

@rowillia Are you still interested in pursuing this? I need to decide whether to release PyAnnotate 1.0.3 with or without it. It will likely be the last release this year due to my vacation schedule and I'd like to release it today or Monday 12/11 at the latest.