gh-119180: Add annotationlib module to support PEP 649
#119891
Conversation
Lib/test/test_annotationlib.py
Outdated
| class NoDict(type): | ||
| @property | ||
| def __dict__(cls): | ||
| raise AttributeError | ||
|
|
||
| class C1(metaclass=NoDict): | ||
| a: int |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
vars(C1) raises an error here already:
>>> class NoDict(type):
... @property
... def __dict__(cls):
... raise AttributeError
...
... class C1(metaclass=NoDict):
... a: int
...
>>> vars(C1)
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
vars(C1)
~~~~^^^^
TypeError: vars() argument must have __dict__ attribute
However, the tests I added don't yet cover cases where we call vars() on the "owner", I'll work on that later.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh! this behaviour changed between two versions actually! (I incorrectly checked it with 3.12.2 actually and not 3.14!)
Um, that sounds bad. We should try to bisect that and make sure it was deliberate.
There was a problem hiding this comment.
Oh no false alarm. How come I got no error yesterday but I got one today? maybe it's an even older version. Let me verify again.
EDIT: Sorry Alex, what I said on the behaviour was incorrect!
There was a problem hiding this comment.
Ah ! I know, actually I was trying to make the error on the type of C1, namely vars(type(C1)) and not on C1 itself. However, it should be possible to have owner = NoDict or is it not possible?
There was a problem hiding this comment.
This actually made me find an even more basic bug:
>>> class X(type):
... a: int
...
>>> class Y(metaclass=X):
... b: str
...
>>> Y.__annotations__
{'a': <class 'int'>}
There was a problem hiding this comment.
Posted https://discuss.python.org/t/pep-749-implementing-pep-649/54974/28 about this, I think I'll want to deal with it in a separate PR (since the solution likely involves changes to #119361).
There was a problem hiding this comment.
For this PR I won't handle the metaclass case yet; still thinking about how to handle that.
|
This PR is still ready for review. While we will likely make some change related to metaclasses, that won't materially effect the changes here. I'd like to get this PR landed as a foundation to build the rest of the PEP implementation on, so I'd appreciate any reviews. |
Lib/annotationlib.py
Outdated
| if self.__forward_module__ is not None: | ||
| globals = getattr( | ||
| sys.modules.get(self.__forward_module__, None), "__dict__", globals | ||
| ) |
There was a problem hiding this comment.
Since this (if set) will override the above heuristics on lines 97-107 anyway, why not move it above line 97 so that if we get a globals from __forward_module__ we don't need to bother with those other lookups?
Lib/test/test_annotationlib.py
Outdated
| def test_expressions(self): | ||
| def f( | ||
| add: a + b, | ||
| sub: a + b, |
There was a problem hiding this comment.
| sub: a + b, | |
| sub: a - b, |
Lib/test/test_annotationlib.py
Outdated
| anno, | ||
| { | ||
| "add": "a + b", | ||
| "sub": "a + b", |
There was a problem hiding this comment.
| "sub": "a + b", | |
| "sub": "a - b", |
| self.assertEqual(annotationlib.get_annotations(int), {}) | ||
| self.assertEqual(annotationlib.get_annotations(object), {}) | ||
|
|
||
| def test_custom_metaclass(self): |
There was a problem hiding this comment.
Do you plan to add tests here for the pathological cases with metaclass annotations, or make that change in a separate PR?
There was a problem hiding this comment.
I'll leave metaclasses to a separate PR (#119180).
Lib/test/test_annotationlib.py
Outdated
|
|
||
| def test_custom_object_with_annotations(self): | ||
| class C: | ||
| def __init__(self, x: int = 0, y: str = ""): |
There was a problem hiding this comment.
Are the arguments and their annotations actually related to the test? If not, I'd remove them for clarity about what behavior the test is actually specifying.
|
Thanks @carljm for the review! I pushed fixes for the issues you identified. |
…n#119891) Co-authored-by: Alex Waygood <[email protected]>
…n#119891) Co-authored-by: Alex Waygood <[email protected]>
This PR implements the
annotationlibmodule proposed by PEP-749, as well as related Python changes for PEP-649 and PEP-749.