don't use __annotations__

[glyph]

attrs/src/attr/_make.py

Line 213 in aa50117

anns = getattr(cls, "__annotations__", {})

uses __annotations__

However, it looks as though an upcoming incompatible change to Python will change __annotations__ into a list of strings.

It seems like the correct public API to use to retrieve the information attrs wants would be get_type_hints.

As a bonus, fixing this might make #265 easier to implement.

[hynek]

I’m very confused. They want to remove typing, but…we have to use typing.get_type_hints() so things keep working!?

Why am I already regretting ever supporting this half-baked bullshit? :'(

[hynek]

Random observation while working on #292: typing.get_type_hints() traverses the whole hierarchy and returns all annotations through all classes. That it going to complicate things.

[glyph]

Wait, what? Where do they want to remove typing?

[hynek]

ericvsmith/dataclasses#61

[mahmoud]

@hynek I've been following that thread and the Python-ideas one, too. I don't think the removal's gonna happen, though at this stage it was certainly disconcerting, even as just a suggestion.

[hynek]

• “I don’t think” not very comforting :|
• importing typing is slooow
• data classes use __annotations__ directly too
• getting the whole hierarchy of annotations is kind of terrible because I'll have to cross-check the annotations with things that have been defined before :| (I cannot just use whatever is in there because the class can have a superclass that isn’t auto_attribs=True).

Looks like I can only lose here. Yay.

[glyph]

Maybe the right thing to do is simply to be aware of this impending change and add version-specific Python support for parsing / evaluating the annotation strings in the future. It does look like it going to have a fairly precise contract.

For that matter, annotations can be strings today; a correct implementation would handle any declaration as a possible forward declaration.

For example,

@attr.s(auto_attribs=True)
class Foo:
    x: 'int'

is a perfectly valid type declaration today, and arguably attr.fields(Foo)[0].type should be int rather than its present value of 'int'.

So perhaps we should just focus on handling forward declarations on current python and then let this shake out naturally in the future version?

[glyph]

Taking that thought further: is this just a dup of #265 ?

[euresti]

Hi. So I've been trying to implement some code that uses the annotation for type checking. And I've run into several interesting issues with __annotations__ and typing.get_type_hints() which I thought would be useful to write about here:

The __annotations__ dictionary can return three types of "objects":

• An honest to goodness fully declared type: e.g. int, List[int], Union[Foo, Bar]
• A type with a forward ref inside it: e.g. List[_ForwardRef('A')]
• A string: 'List[A]'

Example:

>>> class A:
...   a: List[int]
...   b: List['int']
...   c: 'List[int]'
...
>>> A.__annotations__
{'a': typing.List[int], 'b': typing.List[_ForwardRef('int')], 'c': 'List[int]'}

typing.get_type_hints(cls) only succeeds if everything in quotes can be eval-ed with no locals or globals. (Amusingly, get_type_hints(A) above will NameError because it can't find List)

But giving it globals and locals you'd be ok:

>>> typing.get_type_hints(A, globals(), locals())
{'a': typing.List[int], 'b': typing.List[int], 'c': typing.List[int]}

However because of forward and self references you can't guarantee that at class creation type (when attrs is checking for this stuff) typing.get_type_hints() will even succeed.

@attr.s
class B:
    children: List['B']
    other: 'C'

typing.get_type_hints(B, globals(), locals())   # <---- Fails here.

@attr.s
class C:
    parent: Optional[B]

So if the goal is to always get "honest to goodness types" the only way to do it is to save the stack frame in which the class is created, wait until an instance is about to be instantiated, call typing.get_type_hints(cls, frame.f_globals, frame.f_locals) and then overwrite all the types in all the fields.

I've got some hacky code that does that if you want to see it.

[hynek]

Thanks for the writeup! Happy to see your code!

As for the status quo: I think it’s fair game to say that we don’t resolve the types for now however maybe we should document it.

[euresti]

Here's another interesting find. python/cpython@f350a26
Basically it default globals to sys.modules[base.__module__].__dict__

This would help with classes declared at module scope but not with classes declared in functions and such.

Let me see if I can whip my code into shape.

[ambv]

typing.get_type_hints(cls) only succeeds if everything in quotes can be eval-ed with no locals or globals.

This is being somewhat resolved by PEP 563.

[euresti]

So I played around with this for a while and came to the following conclusion. attrs should probably use __annotations__ but have a way to resolve the types to real types if needed.

The real issue is there's no way to guarantee that typing.get_type_hints() won't fail. And about 90% of the time you don't need it. (Technically you need the type for ClassVar but I would just check for "ClassVar" or "typing.ClassVar")

I found three places where you can call typing.get_type_hints()

1. At class creation time. This will work find if there are no forward or self references on the class. Otherwise NameError. (I believe you can hack something to make self references work, by passing {cls.name: cls} to localns.)

2. When a class is instantiated (for example by adding something to init) this works fine for self references and most forward references. Except it won't work if those types are defined in if TYPE_CHECKING: sections.

Here's the code to resolve_types: I believe caching is useful because typing.get_type_hints can be somewhat slow. I don't know how y'all feel about hanging something off the class.

def resolve_types(cls, globalns=None, localns=None):
    """
    Resolve any strings and forward annotations in type annotations.

    :param type cls: Class to resolve.
    :param globalns:
    :param localns:

    :raise TypeError: If *cls* is not a class.
    :raise attr.exceptions.NotAnAttrsClassError: If *cls* is not an ``attrs``
        class.
    :raise NameError: If a cannot be resolved because of missing variables.

    :rtype: True
    """
    try:
        # Since getting type hints is expensive we cache whether we've done
        # it already.
        return cls.__attrs_types_resolved__
    except AttributeError:
        import typing
        hints = typing.get_type_hints(cls, globalns=globalns, localns=localns)
        for field in fields(cls):
            if field.name in hints:
                _obj_setattr(field, 'type', hints[field.name])
        cls.__attrs_types_resolved__ = True
        return cls.__attrs_types_resolved__

This was how I modified _get_annotations to get them at class creation time:

def _get_annotations(cls):
    """
    Get annotations for *cls*.
    """
    anns = getattr(cls, "__annotations__", None)
    if anns is None:
        return {}

    # Verify that the annotations aren't merely inherited.
    for super_cls in cls.__mro__[1:]:
        if anns is getattr(super_cls, "__annotations__", None):
            return {}

    if anns:
        import typing
        try:
            ret = typing.get_type_hints(cls)
            cls.__attrs_types_resolved__ = True
            return ret
        except NameError:
            pass

    return anns

And this is how I check them at instance creation time, by adding it to the generated init.

    if any(a.type for a in attrs):
        lines.append("attr_resolve_types(type(self))")
        names_for_globals["attr_resolve_types"] = resolve_types

[euresti]

I put this code into #302

[hynek]

So um could someone give me an executive summary of all of this? :). Assuming #302 is merged, is there any other immediate need to do something?

FTR, data classes use __annotations__ directly too.

[euresti]

To recap:

The __annotations__ dictionary can return three types of "objects":

• An honest to goodness fully declared type: e.g. int, List[int], Union[Foo, Bar]
• A type with a forward ref inside it: e.g. List[_ForwardRef('A')]
• A string: 'List[A]'
  Note: Once PEP 563 hits __annotations__ will only return strings.

typing.get_type_hints(...) can NameError if strings or ForwardRefs cannot be eval'd. This might be improved in future versions of typing (See python/typing#508) But you might still end up with ForwardRef('...')

Problem 1 Class Generation

This is similar to ericvsmith/dataclasses#92

The current class generating code only cares if the type is a ClassVar, for anything else it only cares that the annotation is there. Amusingly _is_class_var (https://github.com/python-attrs/attrs/blob/master/src/attr/_make.py#L188) handles the string type annotation if it's 'typing.ClassVar[...]' It might be much simpler to just extend it to also handle 'ClassVar[...]' and call it a day. Specially if we don't want to depend/import typing.

Problem 2 The type attribute.

Basically the type attribute in the Attribute(..) ends up coming from __annotations__ and could be any of the three things above. Most users won't care, but for users who want real types instead of strings or ForwardRefs then they are either on their own (the current state) or we bring in #302 to give them a chance.

[hynek]

IOW, we leave this be until the dust in #302 settles, right?

[euresti]

I think so.

[hynek]

If I understand the way #302 was closed correctly, we should tackle this now, right?

[euresti]

I believe the only thing we must do is make sure ClassVar works correctly.

The other part is slightly ambiguous. Do we care what gets put into type? attrs doesn't use it for anything internal. Heck you could write:

x = attr.ib(type='i love types')
y = attr.ib(type=22)

and attrs itself wouldn't care.

If we don't care then we can do nothing. Caring is hard because python/typing#508 is not fixed yet and so you can't safely call get_type_hints() on a class.

[euresti]

Hmm. Actually let me play around with how Python 3.7 will actually work with from __future__ import annotations.

[ambv]

Yeah but you're not wrong that 508 needs addressing before 3.7 is released.

[hynek]

If we don't care then we can do nothing.

❤️❤️❤️

[ambv]

Don't slow down attrs by checking if type= is a type.

[glyph]

If we don't care then we can do nothing.

No open source maintenance best open source maintenance.

[hynek]

Just to circle back to this:

I believe the only thing we must do is make sure ClassVar works correctly.

I does work correctly, right?

So what exactly is the plan for this issue? Anything left to be done or are we gonna keep using __annotations__ until @ambv tells us to stop?

[euresti]

I filed the current ClassVar issue as #361

[mgrandi]

Just found this, which seems to break the cattrs project as it uses functools.singledispatch which obviously doesn't work if the attr.Attribute.type is a string

for now, can we at least update the documentation in either http://www.attrs.org/en/stable/api.html#attr.ib or http://www.attrs.org/en/stable/api.html#attr.Attribute to mention that type can either be the real type or a string?

It took me a while to figure out what was going on and PEP-563 will be landing in a future version of python, so I think it would be valuable to at least have a note about it

[glyph]

Bumping this because we are several python releases closer to this change breaking everything:)

[euresti]

Well it won't break everything. IIUC the only thing that happens is that .type would now return a string. Nothing in attrs itself will be damaged by this. We already handle ClassVar in a decent way. (Though not as hard-core as dataclasses does python/cpython#6768)

That being said 3rd party tools that might want to access the type attribute will start seeing strings and that might confuse them. If we think it's important to support "dereferencing .type then we could bring back #302 though this could easily be done by the 3rd party tool that needs it.

[mgrandi]

I mainly suggested that the docs be updated to have a warning/info box about this

…

On Mon, Feb 17, 2020, 20:53 David Euresti ***@***.***> wrote: Well it won't break everything. IIUC the only thing that happens is that .type would now return a string. Nothing in attrs itself will be damaged by this. We already handle ClassVar in a decent way. (Though not as hard-core as dataclasses does python/cpython#6768 <python/cpython#6768>) That being said 3rd party tools that might want to access the type attribute will start seeing strings and that might confuse them. If we think it's important to support "dereferencing .type then we could bring back #302 <#302> though this could easily be done by the 3rd party tool that needs it. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#288?email_source=notifications&email_token=AAGD7YWHRPV5R2PLUH5PRFDRDNLSHA5CNFSM4EDCYFBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMAPPKI#issuecomment-587265961>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGD7YXE6C2MLJJC6LP32G3RDNLSHANCNFSM4EDCYFBA> .

[Tinche]

So I guess I should add support to cattrs for this? I wonder if there's comprehensive docs anywhere for tools that use runtime types that would help me adjust. Are we just supposed to call get_type_hints() if we encounter a string type? Is that going to be super slow if we do it on every single use?

TBH I feel like the Python typing crowd kinda looks down on using types in runtime, which is amusing because runtime types are several orders of magnitude more useful to me at this point than running mypy over my codebases (which remains borderline useless). My codebases simply wouldn't work at all without runtime types, and the alternatives are much, much uglier and less user-friendly.

[hynek]

I think the right move would indeed be to bring back #302 and add an option to do it automatically?

[euresti]

So the issue is that it can't be done automatically because the types might not be there if there are future references. For example

@attr.s
class Foo:
   bar: "Bar"

@attr.s
class Bar:
    foo: "Foo"

If you try to resolve types when Foo is created you'd fail because Bar doesn't exist. You need to resolve the types for Foo after Bar has been created.

[hynek]

Bleh oh well. Does anything speak against reviving #302?

[hynek]

Heyyy , #302 has been merged!

Any objections against closing this?

[mgrandi]

Sorry I didn't comment, but no objections, it looks good, I'll test it out later once I have a free moment