zopefoundation / zope.annotation

Mechanism for storing additional information about objects without modifying their classes
Other
1 stars 8 forks source link

Investigate the consequences of PEP-563 #15

Open sallner opened 6 years ago

sallner commented 6 years ago

In PEP-563 a system for storing type hints is proposed and the implementation started in Python 3.7. The type hints are stored in __annotations__. It would be helpful to see, whether this could effect somehow the working principle of zope.annotation.

jamadden commented 6 years ago

I've looked at this a little. __annotations__ was introduced way back in PEP-3107 for Python 3. Functions and methods grew the possibility to have an __annotations__ attribute:

Python 3.4.8 (default, Mar 29 2018, 16:18:25)
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo:
...     def thing(a:int) -> int:
...         return 1
...
>>> Foo.thing.__annotations__
{'a': <class 'int'>, 'return': <class 'int'>}

Python 3.6 accepted PEP-563 which let you put annotations on raw variables. Prior to that, only function/method arguments and return types could be annotated AIUI, and I can't find a way to get __annotations__ on a class. But with 3.6, you can now have __annotations__ showing up at class scope:

Python 3.6.2 (v3.6.2:5fd33b5926, Jul 16 2017, 20:11:06)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo:
...     a:int
...
>>> Foo.__annotations__
{'a': <class 'int'>}

PEP 563 only changes the values stored in the annotations dictionary into strings always. It's not enabled by default in 3.7 and requires a __future__ import:

Python 3.7.0rc1 (default, Jun 13 2018, 16:20:06)
[Clang 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from __future__ import annotations
>>> class Foo:
...   a:int
...
>>> Foo.__annotations__
{'a': 'int'}

z.a.attribute.AttributeAnnotations uses __annotations__ and can thus conflict in Python 3.6 and above, leading to false sharing (and failure to persist):

>>> from zope.annotation import attribute
>>> foo = Foo()
>>> attr_ann = AttributeAnnotations(foo)
>>> len(attr_ann)
1
>>> attr_ann['baz'] = 'bar'
>>> foo.__annotations__
{'a': 'int', 'baz': 'bar'}
>>> Foo.__annotations__
{'a': 'int', 'baz': 'bar'}

I'm not sure how to avoid it. The obvious solution is to use vars directly instead of __getitem__, but that breaks in the presence of __slots__, or more generally when a descriptor is used.

jamadden commented 6 years ago

I guess we could add a check like if self.obj.__annotations__ is type(self.obj).__annotations__...

mgedmin commented 6 years ago

Should we rename the attribute to __zope_annotations__ and add backwards-compatibility code to migrate old persistent instances?

freddrake commented 6 years ago

On Thu, Jun 28, 2018 at 10:55 AM Marius Gedminas notifications@github.com wrote:

Should we rename the attribute to __zope_annotations__ and add backwards-compatibility code to migrate old persistent instances?

This gets back to long-standing issue of whether 3rd-party libraries should define their own dunder names. We've lived with the ones we've already defined in the Zope ecosystem for a long time, so changing them as a rule is pretty non-sensical, but here's a case where it can bite.

Guido's long maintained that this practice of the Zope community isn't supported. We should seriously consider selecting a non-dunder name when we do need to deal with something like specific like this, and not add any new Zope-specific dunder names.

Perhaps (z.a) annotations should become _zope_annotations?

-Fred

-- Fred L. Drake, Jr. "A storm broke loose in my mind." --Albert Einstein

icemac commented 1 year ago

Closing this issue as we did not find any problems since the PEP was implemented.

icemac commented 5 months ago

Today I finally came across a problem: Using type annotations on a class which implements IAttributeAnnotatable creates __annotations__ on the class which is writeable and also appears on the instance. When using adapting an instance of such a class to IAnnotation the object on the class is changed!

So never do this:

@zope.interface.implementer(zope.annotation.interfaces.IAttributeAnnotatable)
class C:
    a: str | None = None

inst = C()
IAnnotations(inst)['foo'] = 'bar'

because then:

C.__annotations__ == {'a': str | None, 'foo': 'bar'}
C.__annotations__ is inst.__annotations__

Using Python 3.11 with current zope.annotation version.