[issue27985] Implement PEP 526

[Roundup Robot]

Roundup Robot added the comment:

New changeset ef3d30cc6b4f by Gregory P. Smith in branch 'default':
issue27985 - fix the incorrect duplicate class name in the lib2to3
https://hg.python.org/cpython/rev/ef3d30cc6b4f

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

You're welcome.

> It is first time I make such kind of contribution. It was a great experience 
> and a great opportunity to better understand CPython internals (I already 
> have several ideas on what to work next :-)

If you need any help/mentoring please don't hesitate to approach me directly.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Thank you Guido, Yury, Brett, and Serhiy!

It is first time I make such kind of contribution. It was a great experience 
and a great opportunity to better understand CPython internals (I already have 
several ideas on what to work next :-)

I will soon submit a PR to python/typing with copy of typing changes (also 
Python2 version) and a patch to remove com2ann script to a separate repo as 
discussed with Guido.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

W00t! Thank Ivan for the code! And thanks Yury and Brett for the review.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

Committed.  Congrats Ivan, it's a very serious contribution.  Thank you.

--
resolution:  -> fixed
stage: needs patch -> 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

[issue27985] Implement PEP 526

[Roundup Robot]

Roundup Robot added the comment:

New changeset 49533a9fe322 by Yury Selivanov in branch 'default':
Issue #27985: Implement PEP 526 -- Syntax for Variable Annotations.
https://hg.python.org/cpython/rev/49533a9fe322

--
nosy: +python-dev

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Yes,

This is the latest patch that I tested and with resolved merge conflicts.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

Ivan, is "hg-pep-526-v5.diff" patch the one I can commit?

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

(I've renamed the patches so they line up with the numbering in the code review 
tool.)

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Thank you Yury,

I usually do 

./python -m test -R : test___all__ test_dis test_grammar test_opcodes 
test_parser test_pydoc test_symtable test_tools test_typing

and then

./python -m test -j3 -u all

I just run tests with -R3:3 it also works.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

> Please take a look and sorry for a delay.

Ivan, I'll be the one merging the patch. Will be looking over it soon. I might 
fix some nits myself, so if it applies cleanly to the default branch and all 
tests pass - then I'll handle the rest.

Please also run the tests with -R3:3 (grammar/parser tests specifically)

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Guido,

I resolved merge conflicts in patch v4 (if it will complain, this could be 
because of graminit.c or importlib_external.h, just ignore those, they will be 
regenerated during build).

Please take a look and sorry for a delay.

--
Added file: http://bugs.python.org/file44478/hg-pep-526-v4.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

Ivan, I have no idea how to integrate your patch and Yury's. I only use 
Mercurial here, I don't trust the github clone (how far behind is it?). Please 
sort this out yourself. I am also going through the review on rietveld again.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Oh, I see there are more comments by Serhiy, I will implement them and submit a 
new patch.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Yury, thank you for the fix and for good advice!

(I checked everything like 10 times, except for TOS :-)

I run the full test suite and everything seem to be fine now.

Guido, does Yury's patch apply cleanly, or I need to regenerate a new one?

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

Ivan, take a look at my patch - i've fixed the refleak.  It was in 
STORE_ANNOTATION opcode, you didn't DECREF `ann` consistently in all error 
branches.  Also, you should never "break" in ceval -- only "goto error" or 
"DISPATCH()"

--
Added file: http://bugs.python.org/file44474/hg-pep-526-v3.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Yury,

Commenting out was an attempt to debug. It should be there

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

Yury will give you some help. Also, this patch no longer applies
cleanly to hg. :-(

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Yury Selivanov]

Yury Selivanov added the comment:

> It looks like this part is causing a refleak

There is one DECREF in ceval that you commented out.  I think it should 
actually be there.  But it doesn't solve the problem.

--
nosy: +yselivanov

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

It looks like this part is causing a refleak

def test_do_not_recreate_annotations(self):
class C:
del __annotations__
try: #with self.assertRaises(NameError):
x: int
except NameError:
pass

in test_opcodes

(for both options -- try and with)

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Sorry, again attached a wrong diff, here is the correct one.

--
Added file: http://bugs.python.org/file44470/hg-pep-526-v2.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

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

There is still a refleak to fix.

--
Added file: http://bugs.python.org/file44469/hg-pep-526-v2.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

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

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

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?

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

> 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.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

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.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

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.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

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?

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

I played with the REPL, and found this:

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

I would expect this to re-create __annotations__.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

Guido van Rossum added the comment:

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.

--

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

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

--
Added file: http://bugs.python.org/file44416/hg-pep-526.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Ivan Levkivskyi]

Ivan Levkivskyi added the comment:

Here is the patch for PEP 526 implementation

--
keywords: +patch
nosy: +levkivskyi
Added file: http://bugs.python.org/file44415/hg-pep-526.diff

___
Python tracker 

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

[issue27985] Implement PEP 526

[Guido van Rossum]

New submission from Guido van Rossum:

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

--
assignee: gvanrossum
components: Interpreter Core
messages: 274672
nosy: gvanrossum
priority: normal
severity: normal
stage: needs patch
status: open
title: Implement PEP 526
type: enhancement
versions: Python 3.6

___
Python tracker 

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