classification
Title: Runtime type annotation mutation leads to inconsistent behavior
Type: behavior Stage: resolved
Components: Interpreter Core, Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: eric.smith, ethereon, levkivskyi
Priority: normal Keywords:

Created on 2020-05-10 07:40 by ethereon, last changed 2020-05-16 02:07 by ethereon. This issue is now closed.

Messages (7)
msg368569 - (view) Author: Saumitro Dasgupta (ethereon) Date: 2020-05-10 07:40
Adding type annotations at runtime may lead to inconsistent results. Consider the following example:

    class Base:
        base: int

    class Alpha(Base):
        pass

    class Beta(Base):
        foobar: int

    # Case 1: This mutates Base's __annotations__
    Alpha.__annotations__['injected'] = bool
    assert Alpha.__annotations__ is Base.__annotations__

    # Case 2: This mutates Beta's own copy of __annotations__
    Beta.__annotations__['injected'] = bool

Such mutations of __annotations__ seem to be perfectly legal (https://www.python.org/dev/peps/pep-0526/#runtime-effects-of-type-annotations).

However:
1. In case 1, this leads to the accidental mutation of Base's annotations. Not entirely certain if that's expected, but seems undesirable.

2. There are further differences when looking at `__dict__['__annotations__']`: for Alpha, there is no __annotations__ entry in __dict__. However, for Beta, it's set to `{'foobar': <class 'int'>, 'injected': <class 'bool'>}`. This discrepancy leads to further inconsistent results. In particular, when transforming these classes to dataclasses, which specifically looks at __dict__['__annotations__'](https://github.com/python/cpython/blob/3.8/Lib/dataclasses.py#L856). Converting Alpha to a dataclass leads to no fields. Converting Beta to a dataclass leads to two fields (foobar and injected).  It's worth noting that typing.get_type_hints produces reasonable results here.
msg368976 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2020-05-15 21:10
This is actually a specified behavior (motivated by memory savings for class objects, that are already pretty large). If you scroll down the link you posted it says:

> Note that if annotations are not found statically, then the ``__annotations__`` dictionary is not created at all.


So what do you propose? Changing this behavior is not easy, because it would be a backwards incompatible change.
msg368979 - (view) Author: Saumitro Dasgupta (ethereon) Date: 2020-05-15 21:35
In my opinion, the main problem here is the element of surprise. Given a statement like this:

    foo.__annotations__['injected'] = bool

the expressed intent is "extend this object's annotations". It's surprising that it can sometimes result in accidental mutation of the base's annotations. This surprise may manifest downstream in other parts of the standard library (like dataclasses in the example above), which can be a bit cryptic.

As a performance optimization, it makes sense. However, the element of surprise probably can be improved upon. For instance:

- Explicitly disallow accidental mutation by presenting the parent's dict via a MappingProxy
- Use a "copy-on-write" mechanism
msg368983 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2020-05-15 22:59
But this is no different from every other mutable class variable in Python:

class Base:
    data = {}

class Alpha(Base):
    pass

class Beta(Base):
    data = {}

Alpha.data['injected'] = bool
assert Alpha.data is Base.data

Beta.data['injected'] = bool

I'm not sure what could change here. The choices seem to be break a lot of existing code and have new behavior for all class variables, or do something special for __annotations__.

In general, to get what you want, you'd need to do something like this (going back to your original example):

def add_annotation(cls, v, t):
    if not "__annotations__" in cls.__dict__:
        # Doesn't exist, add it.
        cls.__annotations__ = {}
    cls.__annotations__[v] = t

add_annotation(Base, 'a', int)
add_annotation(Alpha,'a',  float)
add_annotation(Beta, 'a', str)

Which produces:
{'base': <class 'int'>, 'a': <class 'int'>}
{'a': <class 'float'>}
{'foobar': <class 'int'>, 'a': <class 'str'>}

Again, this is just how class variables work in Python.
msg368984 - (view) Author: Saumitro Dasgupta (ethereon) Date: 2020-05-15 23:12
I'd argue that the situation is a bit different from class variables here, since __annotations__ is indirectly brought into existence by the presence of statically-established type annotations. You can be perfectly aware of how class variables work yet find this surprising, since you'd have to be aware of the additional bit of detail pointed out by Ivan above.
msg368985 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2020-05-15 23:26
Perhaps it should be better documented. I don't see the behavior changing.
msg368986 - (view) Author: Saumitro Dasgupta (ethereon) Date: 2020-05-15 23:46
Fair enough. If that's the consensus, I'll close the issue.
History
Date User Action Args
2020-05-16 02:07:10ethereonsetstatus: open -> closed
resolution: wont fix
stage: resolved
2020-05-15 23:46:26ethereonsetmessages: + msg368986
2020-05-15 23:26:10eric.smithsetmessages: + msg368985
2020-05-15 23:12:29ethereonsetmessages: + msg368984
2020-05-15 22:59:38eric.smithsetmessages: + msg368983
2020-05-15 21:35:17ethereonsetmessages: + msg368979
2020-05-15 21:10:32levkivskyisetnosy: + levkivskyi
messages: + msg368976
2020-05-10 12:02:26eric.smithsetnosy: + eric.smith
2020-05-10 07:40:57ethereoncreate