[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

test_grammar also needed a fix. It has been updated to use 
import_helper.import_fresh_module in bpo-43995 (GH-25764).

--
nosy: +erlendaasland

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Ah, I see.  So it wasn't a Windows thing, it was a "we run the test multiple 
times and that particular test assumed it was only run once" thing.  And 
reflink testing is guaranteed to run every test multiple times.

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> So I'm still puzzled about why that test worked on POSIX and failed on Windows

I was able to reproduce it in my MacOS machine (maybe also it reproduced on 
Linux). The problem is that when you run with -R, the test runs several times 
and the import statement is a noop the second time because is cached. We needed 
to modify the test to import the module always

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Obviously the original assertion failure ("AssertionError: False is not true") 
wasn't caused by the refleaks.  So I'm still puzzled about why that test worked 
on POSIX and failed on Windows.  I admit I was pulling some wacky import 
shenanigans in the test, and the import_fresh_module change is an improvement.  
But still... what was going on?!

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset e374a40afa09be728b01653a06c9febfad9c9c50 by Pablo Galindo in 
branch 'master':
bpo-43901: Fix refleaks in test_module (GH-25754)
https://github.com/python/cpython/commit/e374a40afa09be728b01653a06c9febfad9c9c50


--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I made a fix for everything in https://github.com/python/cpython/pull/25754. 
Could you review it?

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

You want a separate PR for the refleak fix, or should I roll them both up into 
one?

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> Eek!  I swear I did a refleak check and got a clean bill of health.  Again, 
> sorry about this!

No problem at all! I'm sure we can fix this on time :)

Opened https://github.com/python/cpython/pull/25754 for the refleaks

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +2
pull_request: https://github.com/python/cpython/pull/25754

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Eek!  I swear I did a refleak check and got a clean bill of health.  Again, 
sorry about this!

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Change by Larry Hastings :


--
pull_requests: +24442
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/25752

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

>> Why would it fail on Windows?

It fails on all refleak buildbots. To reproduce:

% ./configure --with-pydebug -C && make -j -s
% ./python.exe -m test test_module -R :

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Oh, actually is workse because commenting out the test shows refleaks:

㋹ ./python.exe -m test test_module -R :
0:00:00 load avg: 5.03 Run tests sequentially
0:00:00 load avg: 5.03 [1/1] test_module
beginning 9 repetitions
123456789
.
test_module leaked [47, 47, 47, 47] references, sum=188
test_module leaked [6, 6, 6, 6] memory blocks, sum=24
test_module failed

== Tests result: FAILURE ==

1 test failed:
test_module

Total duration: 2.0 sec

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Gee whiz!  Sorry about that.  I swear, it works fine on my machine.

I'll incorporate import_helper.import_fresh_module helper into the test as you 
suggest, and once I get it to work I'll send you a PR.  I don't know how to 
test fixing this failure, though, because again it already works fine on my 
machine.

Why would it fail on Windows?  Are there dramatic differences under the covers 
with respect to the import machinery and caching and such between POSIX and 
Windows?

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I think that test_annotations_are_created_correctly should use the 
import_helper.import_fresh_module helper

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
priority: normal -> release blocker

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Unfortunately commit 2f2b69855d6524e15d12c15ddc0adce629e7de84 has broken the 
buildbots:

==
FAIL: test_annotations_are_created_correctly (test.test_module.ModuleTests)
--
Traceback (most recent call last):
  File 
"D:\buildarea\3.x.ware-win81-release.refleak\build\lib\test\test_module.py", 
line 338, in test_annotations_are_created_correctly
self.assertTrue("__annotations__" in ann_module4.__dict__)
AssertionError: False is not true
--

Example failure:

https://buildbot.python.org/all/#/builders/511/builds/12/steps/4/logs/stdio

--
nosy: +pablogsal
resolution: fixed -> 
status: closed -> open

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Thanks for your feedback and reviews, everybody!  Python just got an eensy 
teensy tiny bit better.

--
assignee:  -> larry
resolution:  -> fixed
stage: patch review -> 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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:


New changeset 2f2b69855d6524e15d12c15ddc0adce629e7de84 by larryhastings in 
branch 'master':
bpo-43901: Lazy-create an empty annotations dict in all unannotated user 
classes and modules (#25623)
https://github.com/python/cpython/commit/2f2b69855d6524e15d12c15ddc0adce629e7de84


--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Jelle Zijlstra]

Jelle Zijlstra  added the comment:

For what it's worth, I checked grep.app and found only one usage of del on 
__annotations__, in a test suite: 
https://github.com/go-python/gpython/blob/master/py/tests/function.py#L82. 
Changing its behavior seems very low risk.

--
nosy: +Jelle Zijlstra

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

I think the PR is in good shape.  If anybody has the time for a review, I'd 
appreciate it!

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Kubilay Kocak]

Change by Kubilay Kocak :


--
components: +Interpreter Core -FreeBSD
nosy:  -koobs

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[ANDRZEJ J.]

Change by ANDRZEJ J. :


--
components: +FreeBSD -Interpreter Core
nosy: +kokik, koobs

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Change by Larry Hastings :


--
keywords: +patch
pull_requests: +24322
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/25623

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Guido van Rossum]

Guido van Rossum  added the comment:

I think "no user-visible changes" is a pipe dream. Deleting __annotations__ 
seems fairly pointless so I don't mind changes that are only visible when you 
do that.

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

> Functions don't store __annotations__ in their __dict__, it is a
> separate slot named func_annotations (see funcobject.c). I guess
> that's because the __dict__ is purely for user-defined function
> attributes.

I brought up functions because I'm now proposing to make classes and modules 
behave more like functions in this one way, that they too will now lazy-create 
an empty annotations dict every time.  But yes, function objects don't store 
__annotations__ in their __dict__, and best practices for 3.9 and before was to 
use fn.__annotations__ or getattr() when getting the annotations from a 
function object, and 3.10 will not change that best practice.

I don't know specifically why the implementor made that design choice, but I 
might have done that too.  Most function objects don't have a __dict__ so this 
is almost always cheaper overall.  And recreating the dict seems harmless.

(Just to round the bases on this topic: I don't think you should be permitted 
to delete __annotations__, like most other metadata items on functions / 
classes / modules.  But I don't propose to change that for 3.10.)


> So if you look in __dict__ it will be like it's still Python 3.9,
> but if you're using the attribute (the recommended approach for code
> that only cares about 3.10) it'll be as if it always existed. Sounds
> pretty compatible to me.

Yes, exactly.  That was the thing I finally figured out this afternoon.  Sorry 
for being a slow learner.

Again, this approach will change the semantics around deleting annotations on 
class and module objects.  Deleting them won't be permanent--if you delete one, 
then ask for it, a fresh one will be created.  But that seems harmless.


> So, honestly I don't understand what your concern with the lazy
> approach is. Was your design based on having a bit in the
> class/module object (outside its __dict__) saying "I already
> lazily created one"? Or am I missing something?

My concern is that always lazy-creating on demand will change user-visible 
behavior.  Consider this code:

class C:
a:int=3

del C.__annotations__
print(C.__annotations__)

In 3.9, that throws an AttributeError, because C no longer has an 
'__annotations__' attribute.  If I change Python 3.10 so that classes and 
modules *always* lazy-create __annotations__ if they don't have them, then this 
code will succeed and print an empty dict.

That's a user-visible change, and I was hoping to avoid those entirely.  Is it 
a breaking change?  I doubt it.  Is it an important change?  It doesn't seem 
like it.  I bring it up just in the interests of considering every angle.  But 
I don't think this is important at all, and I think always lazy-creating 
annotations dicts on classes and modules is the right approach.

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Guido van Rossum]

Guido van Rossum  added the comment:

So, honestly I don't understand what your concern with the lazy approach is. 
Was your design based on having a bit in the class/module object (outside its 
__dict__) saying "I already lazily created one"? Or am I missing something?

Also, I'll stop going on about "best practice".

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Guido van Rossum]

Guido van Rossum  added the comment:

Functions don't store __annotations__ in their __dict__, it is a separate slot 
named func_annotations (see funcobject.c). I guess that's because the __dict__ 
is purely for user-defined function attributes.

But perhaps for classes the C equivalent of this pseudo-code will work?

@property
def __annotations__(self):
if "__annotations__" not in self.__dict__:
self.__dict__["__annotations__"] = {}
return self.__dict__["__annotations__"]

The whole thing is protected by the GIL of course, so there's no race condition 
between the check and the assignment.

So if you look in __dict__ it will be like it's still Python 3.9, but if you're 
using the attribute (the recommended approach for code that only cares about 
3.10) it'll be as if it always existed. Sounds pretty compatible to me.

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Larry Hastings  added the comment:

Hmm.  Sorry for the stream-of-consciousness thought process here, but this 
approach adds wrinkles too.

Function objects from the very beginning have lazy-created their annotations 
dict if it's not set.  Which means this works fine:

while True:
del fn.__annotations__
print(fn.__annotations__)

You can do that all day.  The function object will *always* create a new 
annotations object if it doesn't have one.  del fn.__annotations__ always 
works, and accessing fn.__annotations__ always succeeds.  It doesn't retain any 
state of "I already lazily created one, I shouldn't create another".

If I add getsets to classes and modules so they lazy-create annotations on 
demand if they otherwise aren't set, then either a) they need an extra bit set 
somewhere that says "I lazily created an empty annotations dict once, I 
shouldn't do it again", or b) they will duplicate this behavior that functions 
display.

a) would better emulate existing semantics; b) would definitely be a 
user-visible breaking change.  There's actually a test in the 
Lib/test/test_opcodes (iirc) that explicitly tests deleting __annotations__, 
then checks that modifying the annotations dict throws an exception.  I haven't 
done any investigating to see if this check was the result of a regression, and 
there was a bpo issue, and there was a valid use case, etc etc etc.

My instinct is that deleting o.__annotations__ is not an important or 
widely-used use case.  In fact I plan to recommend against it in my "best 
practices" documentation.  So either approach should be acceptable.  Which 
means I should go with the simpler one, the one that will duplicate the 
function object's always-recreate-annotations-dicts behavior.

--

___
Python tracker 

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

[issue43901] Lazy-create an empty annotations dict in all unannotated user classes and modules

[Larry Hastings]

Change by Larry Hastings :


--
title: Add an empty annotations dict to all unannotated classes and modules -> 
Lazy-create an empty annotations dict in all unannotated user classes and 
modules

___
Python tracker 

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