[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Saumitro Dasgupta]

Change by Saumitro Dasgupta :


--
resolution:  -> wont fix
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Saumitro Dasgupta]

Saumitro Dasgupta  added the comment:

Fair enough. If that's the consensus, I'll close the issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Eric V. Smith]

Eric V. Smith  added the comment:

Perhaps it should be better documented. I don't see the behavior changing.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Saumitro Dasgupta]

Saumitro Dasgupta  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Eric V. Smith]

Eric V. Smith  added the comment:

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': , 'a': }
{'a': }
{'foobar': , 'a': }

Again, this is just how class variables work in Python.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Saumitro Dasgupta]

Saumitro Dasgupta  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Ivan Levkivskyi]

Ivan Levkivskyi  added the comment:

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.

--
nosy: +levkivskyi

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Eric V. Smith]

Change by Eric V. Smith :


--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40583] Runtime type annotation mutation leads to inconsistent behavior

[Saumitro Dasgupta]

New submission from Saumitro Dasgupta :

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': , 'injected': }`. 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.

--
components: Interpreter Core, Library (Lib)
messages: 368569
nosy: Saumitro Dasgupta
priority: normal
severity: normal
status: open
title: Runtime type annotation mutation leads to inconsistent behavior
type: behavior
versions: Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com