Implement PEP 526

[gvanrossum]

BPO	27985
Nosy	@gvanrossum, @1st1, @ilevkivskyi
Files	hg-pep-526.diff: (ignore) hg-pep-526-v2a.diff hg-pep-526-v3.diff: (ignore) hg-pep-526-v4.diff hg-pep-526-v4a.diff hg-pep-526-v5.diff: (Renamed to v5 to match code review labeling)

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2016-09-09.03:54:03.020>
created_at = <Date 2016-09-06.23:48:46.989>
labels = ['interpreter-core', 'type-feature']
title = 'Implement PEP 526'
updated_at = <Date 2016-09-09.21:48:16.681>
user = 'https://github.com/gvanrossum'

bugs.python.org fields:

activity = <Date 2016-09-09.21:48:16.681>
actor = 'python-dev'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2016-09-09.03:54:03.020>
closer = 'yselivanov'
components = ['Interpreter Core']
creation = <Date 2016-09-06.23:48:46.989>
creator = 'gvanrossum'
dependencies = []
files = ['44415', '44416', '44469', '44470', '44474', '44478']
hgrepos = []
issue_num = 27985
keywords = ['patch']
message_count = 33.0
messages = ['274672', '274693', '274695', '274914', '274918', '274919', '274925', '274929', '274930', '274938', '274954', '275079', '275080', '275081', '275083', '275085', '275088', '275097', '275115', '275117', '275136', '275158', '275163', '275169', '275184', '275191', '275192', '275245', '275246', '275247', '275265', '275267', '275458']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'python-dev', 'yselivanov', 'levkivskyi']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27985'
versions = ['Python 3.6']

[gvanrossum]

Pending PEP-526's acceptance, I am inviting Ivan Levkivskyi to upload his patch here so it can be reviewed.

[ilevkivskyi]

Here is the patch for PEP-526 implementation

[ilevkivskyi]

Oops, sorry, forgot to add new files when converting from git to hg, here is the full patch.

[gvanrossum]

Hey Ivan, Brett and I divided the review, he started at the bottom and I started at the top, we're meeting at Lib/typing.py.

[gvanrossum]

I played with the REPL, and found this:

>>> del __annotations__
del __annotations__
>>> x: int = 0
x: int = 0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: __annotations__ not found
>>> 

I would expect this to re-create __annotations__.

[ilevkivskyi]

We discussed this at some point on python/typing. At that time we decided that

class C:

del __annotations__
x: int

Should be an error. Do you think that it should behave in a different way in interactive REPL and/or at module level?

[ilevkivskyi]

I could change STORE_ANNOTATION opcode so that it will recreate __annotations__ if __name__ == '__main__'.

Or do you now think that it should re-create it always? I still think that always re-creating __annotations__ if they don't exist seems like silencing a possible error. As I mentioned in previous discussion, I think we should allow people to explicitly del __annotations__ (for example if someone wants to make a class with annotations that are "invisible" to runtime tools) and warn them if later they use annotations.

[gvanrossum]

Each statement at the REPL should re-initialize __annotations__ if it
contains any annotations. I think this is how exec() already works. It
adds __annotations__ to the namespace as needed, but just updates it
if present. Inside a class it's different, that should be considered a
single block.

[ilevkivskyi]

I think this is how exec() already works

This is not exactly like this, exec() emits ast.Module while interactive input emits ast.Interactive (this one skips compiler_body in compile.c), but I think I have got your point, will fix this.

[ilevkivskyi]

Guido,

I fixed __annotations__ in interactive REPL, will fix other minor things and submit a new patch tomorrow morning.

There are two important questions in typing.py:

Why is the second isinstance() needed? Isn't _ClassVar a subclass of
TypingMeta

and

This being a metaclass, the name should end in Meta, like most
other subclasses of TypingMeta. (And they don't have a leading _,
though they're still not public.)

The problem with ClassVar is that it is not actually a class, and _ClassVar is not a metaclass, it is just a class. I deviated from the common pattern in module for the following reason. ClassVar is going to be extensively used at class scope, where all annotations are evaluated. Therefore I wanted not to create new class objects by __getitem__ like it is going for other classes like Union etc (I tried to timeit this and ClassVar is almost ten times faster than Union)

What do you think?
Should I keep it like this, or rewrite in a common pattern for the module?

[gvanrossum]

Oh, dang, I misread the definition of _ClassVar. It's fine then!
Looking forward to the next installment.

[ilevkivskyi]

Here is the new patch. I hope I didn't miss any comment and fixed everything.

There is still a refleak to fix.