PEP 749: Add section on metaclasses #3847

JelleZijlstra · 2024-06-19T05:08:39Z

Deriving from https://discuss.python.org/t/pep-749-implementing-pep-649/54974/28 and offline discussion with Alex.

📚 Documentation preview 📚: https://pep-previews--3847.org.readthedocs.build/

peps/pep-0749.rst

Co-authored-by: Carl Meyer <carl@oddbird.net>

ncoghlan

LGTM over all, but a couple of questions about implementation details inline.

ncoghlan · 2024-06-19T07:48:59Z

peps/pep-0749.rst

+and replaces itself with the result. The descriptor also behaves like a mapping,
+so that code that uses ``cls.__dict__["__annotations__"]`` will still usually
+work: treating the object as a mapping will evaluate the annotations and behave
+as if the descriptor itself was the annotations dictionary. (Code that assumes


Will the implicit mapping access also update the cls.__dict__ entry? (thanks to the owner information passed to __set_name__, it could).

If it doesn't, the specification should be explicit that the underlying dict will be cached on the descriptor instance, so it can be added to cls.__dict__ later and any changes made via the descriptor's mapping API will still be visible.

I have a pure-Python proof of concept for the idea here: https://gist.github.com/AlexWaygood/29e386e092377fb2e288620df1765ed5

The PoC does not currently update the __dict__ entry -- it just caches the materialized annotations internally in the descriptor instance -- but I think you're right that it could, potentially.

Adding a sentence that makes my proposal more explicit.

ncoghlan · 2024-06-19T07:52:02Z

peps/pep-0749.rst

+retrieve annotations.
+
+Alex Waygood has suggested an approach that avoids these problems. On class
+creation, ``cls.__dict__["__annotations__"]`` is set to a special descriptor.


Do type.__annotate__ and type.__annotations__ still exist in this approach? (I don't believe they're needed, but best to be explicit)

I agree that I don't think they'd be needed with this approach

I believe they may be needed for non-heap types. I will experiment with this when I implement the idea.

ncoghlan · 2024-06-19T07:57:24Z

peps/pep-0749.rst

+class object returns ``None`` if the class has no annotations, and an annotation
+function that returns the class's annotations if it does have annotations.
+
+Classes always contain ``__annotations__`` and ``__annotate__`` keys in their


Does "classes" here mean all type instances? Or specifically heap types created with a class statement? What about the equivalent Python and C APIs?

I suspect only types created via a class statement getting these descriptors implicitly will make the most sense, with static types and types created via the dynamic Python and C APIs getting __annotate__ = None and __annotations__ = {} if nothing else is specified in the supplied class dicts.

All classes that have >=1 class with annotations in their MRO must have "__annotations__" and "__annotate__" keys in their class dictionaries under this proposal. Theoretically I think it would be fine to omit these keys from the class dictionary if you could reliably determine that no classes in the MRO have any annotations. I don't know if that's worth the complexity, though.

All classes that have >=1 class with annotations in their MRO

Or in the MRO of a metaclass.

Does "classes" here mean all type instances? Or specifically heap types created with a class statement? What about the equivalent Python and C APIs?

I think you're right and this should apply to classes created through the class statement, and classes created in ways that may inherit from Python classes should simply set the fields to None and {}.

All classes that have >=1 class with annotations in their MRO

Or in the MRO of a metaclass.

right, yeah

AlexWaygood

Nice

peps/pep-0749.rst

AlexWaygood · 2024-06-19T11:37:11Z

peps/pep-0749.rst

+class object returns ``None`` if the class has no annotations, and an annotation
+function that returns the class's annotations if it does have annotations.
+
+Classes always contain ``__annotations__`` and ``__annotate__`` keys in their


All classes that have >=1 class with annotations in their MRO must have "__annotations__" and "__annotate__" keys in their class dictionaries under this proposal. Theoretically I think it would be fine to omit these keys from the class dictionary if you could reliably determine that no classes in the MRO have any annotations. I don't know if that's worth the complexity, though.

AlexWaygood

LGTM

carljm · 2024-06-19T14:55:41Z

After sleeping on this and looking at the proof of concept in pure Python, I'm a bit concerned about the performance implications. Classes are already large, but adding a new object instance with several fields to every class that exists could still have serious memory consequences for large Python codebases. I think we need to consider whether the bugs that are being fixed here are serious enough in practice to justify this overhead, or whether there are simpler solutions that don't fix everything perfectly but address the biggest problems, and don't have this overhead.

JelleZijlstra · 2024-06-19T15:02:00Z

@carljm it would be a small proportion of the overall size of a class object. In the prototype I'm currently working on, the size of the descriptor object is just 48 bytes, only 3% of the size of the class object:

>>> class X: a: int
... 
>>> ann = X.__dict__["__annotations__"]
>>> ann.get("a")
<class 'int'>
>>> ann.get("b")
>>> import sys
>>> sys.getsizeof(X)
1712
>>> sys.getsizeof(ann)
48

JelleZijlstra · 2024-06-19T15:19:40Z

python/cpython#120719 has a very partial prototype now (I haven't gotten around to testing it yet or integrating descriptor behavior).

carljm · 2024-06-19T18:58:20Z

What if we never set __annotations__ in the dictionary of any subclass of type? This would mean that introspecting annotations on a metaclass would always have to re-run __annotate__, but I think it would solve the problems described here?

Or alternatively, if we used the "cache annotations under a different name than __annotations__" strategy, but only for subclasses of type.

ncoghlan · 2024-06-19T23:49:22Z

The concern with the "cache under a different name strategy" is that it isn't fully backwards compatible: retrieving the annotations directly from the class dict wouldn't work anymore.

JelleZijlstra · 2024-06-20T01:04:25Z

The concern with the "cache under a different name strategy" is that it isn't fully backwards compatible: retrieving the annotations directly from the class dict wouldn't work anymore.

That's also true for the original text of PEP 649. It says:

Class and module objects must cache the annotations dict in their dict, using the key annotations. This is required for backwards compatibility reasons.

But that's not really true under PEP 649 as written: __dict__["__annotations__"] only exists if you previously warmed the cache by accessing the .__annotations__ descriptor. Arguably that's worse than removing __annotations__ completely from the dict: now whether or not __dict__["__annotations__"] works is path-dependent.

This makes me think we should either use a strategy where __dict__["__annotations__"] continues to work reliably (Alex's proposal, now in this PR), or we should go for a clean break and never set __annotations__ in the dict. If so, we should probably do the same for modules.

It also occurred to me that we may want to avoid using the class dict as storage for __annotate__. In the future, we may want to make optimizations so that __annotate__ is not stored as a function, but as just a code object that gets lazily materialized into a function. If we guarantee that cls.__dict__["__annotate__"] is a function, that optimization becomes harder to achieve.

Or alternatively, if we used the "cache annotations under a different name than __annotations__" strategy, but only for subclasses of type.

We could do something like this, but I'd rather have all classes work similarly, instead of special-casing some categories of classes.

AlexWaygood · 2024-06-20T10:28:33Z

In the proof-of-concept gist currently, @carljm is correct that every class -- even if it has no annotations -- gets a unique descriptor instance in its class dictionary:

class Object:
    """Pretend this is how builtins.object would work"""

    __annotate__ = None
    annotations = {}

    def __init_subclass__(cls):
        cls.annotations = _AnnotationsDescriptor(cls)
        if "__annotate__" not in cls.__dict__:
            cls.__annotate__ = None

But this does indeed seem unnecessary for classes that have no annotations. Instead, we could have a single descriptor instance in the class dictionary for builtins.object that is reused for all classes with no annotations:

class Object:
    """Pretend this is how builtins.object would work"""

    __annotate__ = None

    def __init_subclass__(cls):
        if "__annotate__" in cls.__dict__:
            cls.annotations = _AnnotationsDescriptor(cls)
        else:
            cls.__annotate__ = None
            cls.annotations = Object.__dict__["annotations"]


Object.annotations = _AnnotationsDescriptor(Object)

This should result in dramatically fewer descriptor instances being produced at class creation time. We would still create a fresh descriptor instance for every class that does have annotations and insert that instance into the class dictionary; but this is not so different to the way that an __annotations__ dictionary is inserted into the class dictionary for these classes in Python 3.13 anyway.

JelleZijlstra · 2024-06-20T14:48:19Z

@AlexWaygood that becomes problematic if the __annotations__ dict is modified. I can see a few options:

There is a single shared dict used by all unannotated classes. But now modifying that dict on one class affects all other unannotated classes.
Every time the descriptor is accessed, it returns a fresh dict. But now if you modify a class's __annotations__, the changes won't be preserved if you access __annotations__ again.
There is a lazily-created annotations dict for every class. But that means we need a per-class place to put it, and we're back where we started.
Instead of a mutable dict we return an immutable mappingproxy. Probably the best option, but still breaks use cases that rely on modifying the annotations dict.

carljm · 2024-06-20T22:30:26Z

It also occurred to me that we may want to avoid using the class dict as storage for __annotate__. In the future, we may want to make optimizations so that __annotate__ is not stored as a function, but as just a code object that gets lazily materialized into a function. If we guarantee that cls.__dict__["__annotate__"] is a function, that optimization becomes harder to achieve.

This is a really good point. That optimization was already implemented in the prototype implementation of PEP 649 that was available at the time of its acceptance, and although the optimization wasn't specified in the text of PEP 649, it was discussed publicly in advocating for the PEP prior to its acceptance, and I think it may turn out to be a valuable optimization. So it would be great if we can avoid closing the door to it.

JelleZijlstra · 2024-07-21T02:54:40Z

Coming back to this after a month, I'm now leaning towards the approach that the current version of this PR rejects:

Bypass normal attribute lookup when accessing these attributes, instead
invoking the base class descriptor (from e.g., type.__dict__["__annotations__"])
directly.

This approach has the disadvantage that odd things may still happen if you access .__annotations__ directly, but on the other hand it can be implemented without adding any additional complexity or behavior changes to normal classes.

The details would be that we'd use this trick in annotationlib.get_annotations, and document clearly that accessing .__annotations__ directly on a class is unsafe.

This also implies we should add an annotationlib.get_annotate to safely return the __annotate__ function in the presence of metaclasses.

See python/peps#3847 (comment)

carljm

This looks like a reasonable compromise to me.

Co-authored-by: Carl Meyer <carl@oddbird.net>

@Carreau

* Include three comments from @Carreau (drop two bullets, *args in example, explain *args.) * Lambda-wrapped expressions use annotation scope * Clarify use of annotation scope * Mention what happens to named unicodes followed by text * Use DecodedConcrete in assertion * Rewrite why annotation scope is needed (#4) * Rewrite why annotation scope is needed * Minor copyediting * PEP 747: Fix rules related to UnionType (T1 | T2). Contrast TypeExpr with TypeAlias. Apply other feedback. (python#3856) * PEP 694: Fix typo (python#3859) * PEP 2026: Update following discussion (python#3860) Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> * PEP 101: Remove outdated info and add new info (python#3863) * PEP 101: Remove outdated info * PEP 101: Update make command for running tests * PEP 101: Replace '#python-dev and/or python-committers' with 'Discord and/or Discourse * PEP 101: Add Hugo as 3.14 RM * PEP 101: Add to PSRT * PEP 11: Add Russell as an iOS contact (python#3865) * Meta: Document the PEPs API (python#3864) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> * PEP 719: Update for today's release of 3.13.0b4 (python#3868) * PEP 740: Mark as Provisional (python#3848) Signed-off-by: William Woodruff <william@yossarian.net> * PEP 749: Add section on metaclasses (python#3847) Co-authored-by: Carl Meyer <carl@oddbird.net> * PEP 8: Update a Wikipedia link (python#3552) * PEP 635: Minor typo fix in code sample (python#3871) Looks like an unclosed f-string. * PEP 751: A file format to list Python dependencies for installation reproducibility (python#3870) Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> Co-authored-by: Carol Willing <carolcode@willingconsulting.com> * PEP 743: Rewrite to hide (soft-)deprecated API (pythonGH-3869) Co-authored-by: Victor Stinner <vstinner@python.org> * PEP 751: Add Discussions-To and Post-History (python#3872) * PEP 639: Incorporate the latest discussion feedback (python#3866) * Remove the requirement of license-files defaults * Cover all rejected subkeysideas in one paragraph * Change the deprecation policy around classifiers * Flatten the value of the license-files key, only globs are specified * Update the Rejected ideas to match the current license-files proposal --------- Co-authored-by: Miro Hrončok <miro@hroncok.cz> * PEP 715: clarify what `[package.tool]` is (python#3873) * PEP 665: Superseded-By: 751 (python#3875) * PEP 751: update based on feedback (python#3877) * PEP 751: update based on feedback * Fix a section underline * Include three comments from @Carreau (drop two bullets, *args in example, explain *args.) * From Carol, move the point about import to the following paragraph. * Per Carol: Remove paragraph about lifecycles as that is about *a* DSL, not DSLs in general. --------- Signed-off-by: William Woodruff <william@yossarian.net> Co-authored-by: pauleveritt <paul.everitt@jetbrains.com> Co-authored-by: Jim Baker <jim.baker@python.org> Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com> Co-authored-by: David Foster <david@dafoster.net> Co-authored-by: Barry Warsaw <barry@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: T. Wouters <thomas@python.org> Co-authored-by: William Woodruff <william@yossarian.net> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> Co-authored-by: Carl Meyer <carl@oddbird.net> Co-authored-by: Lavrentiy Rubtsov <rnbsov@gmail.com> Co-authored-by: Mariatta <Mariatta@users.noreply.github.com> Co-authored-by: Brett Cannon <brett@python.org> Co-authored-by: Carol Willing <carolcode@willingconsulting.com> Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Karolina Surma <33810531+befeleme@users.noreply.github.com> Co-authored-by: Miro Hrončok <miro@hroncok.cz>

PEP 749: Add section on metaclasses

bc211ee

JelleZijlstra requested review from ncoghlan and AlexWaygood June 19, 2024 05:08

carljm reviewed Jun 19, 2024

View reviewed changes

peps/pep-0749.rst Outdated Show resolved Hide resolved

Update peps/pep-0749.rst

629a50b

Co-authored-by: Carl Meyer <carl@oddbird.net>

ncoghlan approved these changes Jun 19, 2024

View reviewed changes

AlexWaygood reviewed Jun 19, 2024

View reviewed changes

code review and expansions

bdd6198

AlexWaygood approved these changes Jun 19, 2024

View reviewed changes

JelleZijlstra mentioned this pull request Jun 21, 2024

gh-119180: Alternative approach to metaclass annotations: never use the dict python/cpython#120816

Closed

Third approach

ef781a5

JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this pull request Jul 21, 2024

pythongh-119180: Yet another approach for fixing metaclass annotations

c87cccd

See python/peps#3847 (comment)

JelleZijlstra mentioned this pull request Jul 21, 2024

gh-119180: Use type descriptors to access annotations (PEP 749) python/cpython#122074

Merged

carljm approved these changes Jul 22, 2024

View reviewed changes

JelleZijlstra merged commit c1c52a5 into python:main Jul 23, 2024
6 checks passed

JelleZijlstra deleted the pep749-classanno branch July 23, 2024 20:50

pauleveritt pushed a commit to pauleveritt/peps that referenced this pull request Aug 6, 2024

PEP 749: Add section on metaclasses (python#3847)

e248a85

Co-authored-by: Carl Meyer <carl@oddbird.net>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

PEP 749: Add section on metaclasses #3847

PEP 749: Add section on metaclasses #3847

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PEP 749: Add section on metaclasses #3847

PEP 749: Add section on metaclasses #3847

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!