GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-137530: generate an __annotate__ function for dataclasses __init__ by DavidCEllis · Pull Request #137711 · python/cpython

Conversation

@DavidCEllis

@DavidCEllis DavidCEllis commented Aug 13, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This is one approach to resolving #137530

It generates a new __annotate__ function to attach to the __init__ method and removes the in-line annotations that are now unused.

It's possible to keep the original source annotations if necessary, but they provide a second possibly incorrect source of the information and they will probably also cause additional issues with #135228 as they may keep the original class around when ForwardRef values are generated.

The generated __annotate__ will also keep these references and need to be regenerated. I can extend this or follow up to handle these references but I didn't want to do too much in one PR.

  • Issue: Calling help() on a dataclass shows dataclass internal names instead of type names in 3.14 or later #137530

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
DavidCEllis added 6 commits August 12, 2025 18:16
@DavidCEllis
replace source annotations in init with attached annotate function
5820f94
@DavidCEllis
Include the new test for init annotations in all formats
d0c5d3b
@DavidCEllis
Add an extra check for init=False attributes
b340a2f
@DavidCEllis
Test that defining a forwardref makes init annotations resolve
c7a4c56
@DavidCEllis
move _make_annotate_function to top level
3a3b6c0
needed for _add_slots
@DavidCEllis
stop unpacking a dictionary into another dictionary
13b11c1
@DavidCEllis
DavidCEllis requested a review from ericvsmith as a code owner August 13, 2025 14:22
@bedevere-app bedevere-app Bot added the awaiting review label Aug 13, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Aug 13, 2025
Calling help() on a dataclass shows dataclass internal names instead of type names in 3.14 or later #137530
Closed
JelleZijlstra
JelleZijlstra reviewed Aug 15, 2025
View reviewed changes
Comment thread Lib/dataclasses.py Outdated
elif isinstance(v, annotationlib.ForwardRef):
string_annos[k] = v.evaluate(format=annotationlib.Format.STRING)
else:
string_annos[k] = annotationlib.type_repr(v)

@JelleZijlstra JelleZijlstra Aug 15, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit unfortunate because we could get better STRING annotations by calling with the STRING format on the original class (and its base classes).

An approach to do this would be to make __init__.__annotate__ delegate to calling get_annotations() on the class itself, and on any base classes that are contributing fields, with the appropriate format, then processing the result to add "return": None and filter out any fields that don't correspond to __init__ parameters.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@DavidCEllis DavidCEllis Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did think about that, but wasn't certain about the extra complexity, but I've since noticed that if there's a hint that contains a ForwardRef (like list[undefined]) the result leaks all the ForwardRef details.

I've added a test for that example to demonstrate why this is necessary.

I've taken a slightly different approach, in that I'm gathering all inherited annotations and using them to update the values from the fields. The original source annotation logic this replaces wasn't __init__ specific so I've tried to keep the __annotate__ generator non-specific too.


There are additional commits as this change meant that __annotate__ now (always) has a reference to the original class which broke #135228 - this now also includes an additional test and fix for the issue I brought up there as I already needed to update the fields for this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@DavidCEllis DavidCEllis Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also unintentionally discovered that the __annotate__ functions CPython generates for methods have an incorrect __qualname__ while fixing the __qualname__ for the function generated here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
DavidCEllis added 5 commits August 15, 2025 10:07
@DavidCEllis
Get better 'Source' annotations - break slots gc tests
dca9a5b
@DavidCEllis
Add an extra GC test for forwardrefs
a4fe18a
@DavidCEllis
Remove references to original class from fields and annotate functions
7795c9b
@DavidCEllis
Test we don't fix annotations on hand-written init functions
37d9b3c
@DavidCEllis
*Actually* test we don't fix annotations on hand-written init functions
864305d
The possibility to replace only occurs in _add_slots.
@DavidCEllis DavidCEllis mentioned this pull request Aug 15, 2025
Better string annotations results from generated annotate methods DavidCEllis/ducktools-classbuilder#51
Merged
@JelleZijlstra
JelleZijlstra self-requested a review August 15, 2025 13:34
@DavidCEllis

DavidCEllis commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

On doing some other work, I think the logic for STRING is also needed for FORWARDREF. I'm not sure how to resolve VALUE.

Problem for both:

from annotationlib import get_annotations
from dataclasses import dataclass

@dataclass
class Example:
    a: list[defined_late]

defined_late = int

print(get_annotations(Example.__init__))
{'a': list[ForwardRef('defined_late', is_class=True, owner=<class '__main__.Example'>)], 'return': None}

For Format.FORWARDREF this is resolvable in the same way as STRING. However for VALUE it doesn't quite work out to do the same thing, as the annotations on the class could contain a name that doesn't exist yet - but if init=False that name may not be required for the __init__ annotations.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DavidCEllis

DavidCEllis commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

Ah, while this is resolvable for FORWARDREF - it requires VALUE to fail or get_annotations will use VALUE instead anyway.

I'm leaning towards making VALUE fail if any of the class annotations fail with VALUE. I can't currently think of a clean way otherwise to both make FORWARDREF evaluate as far as possible and not leak ForwardRef values into VALUE annotations.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DavidCEllis
Use the same annotation logic for VALUE, FORWARDREF and STRING
9c6ed47
Also split out the single large test into multiple smaller tests
@DavidCEllis

DavidCEllis commented Aug 15, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Thoughts:

  • With the change to collect class annotations, all of the values from the annotations argument are no longer used apart from the return type.
    • Perhaps _make_annotate_function should take (cls, annotation_fields, return_type) instead of (cls, annotations), where annotation_fields is just a list of relevant field names?
    • _FuncBuilder.add_fn would regain return_type and replace annotations with annotation_fields
  • I'd like to make the VALUE annotations work in test_init_false_forwardref but I don't currently see a clean way to do so.

Edit: I realised changing the signature would make _add_slots simpler and is probably worthwhile.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

DavidCEllis added 2 commits August 15, 2025 18:26
@DavidCEllis
Use a list of fields and return type instead of annotations dictionary.
ab74435
Use `__class__` as the first argument name so `_update_func_cell_for__class__` can update it.
@DavidCEllis
remove outdated comment
30512bd
@DavidCEllis

DavidCEllis commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

Ok, I'm going to try to leave this alone for some review.

The only functional1 thing I know I'm not completely happy with is this example from the tests2:

from dataclasses import dataclass, field
from annotationlib import get_annotations

@dataclass
class F:
    not_in_init: list[undefined] = field(init=False, default=None)
    in_init: int

annos = annotationlib.get_annotations(F.__init__)  # NameError 

I don't have a good approach for this. I don't think you can use FORWARDREF annotations as they are without potentially leaking actual ForwardRefs?

Footnotes

  1. The first parameter of _make_annotate_function being named __class__ in order to reuse the cell update function could maybe be cleaner. ↩

  2. Ignore the incorrect default type, initially the annotation had | None and I forgot to remove the default when I removed it. ↩

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DavidCEllis

DavidCEllis commented Aug 25, 2025

Copy link
Copy Markdown
Contributor Author

The ideal solution I would like for this would require an additional feature from annotationlib to provide completely unevaluated references.

Then a solution more like the original version of this PR could work.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Oct 20, 2025
View reviewed changes
Comment thread Lib/test/test_dataclasses/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Oct 20, 2025
View reviewed changes
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Oct 20, 2025
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, except for the small suggestions above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
DavidCEllis added 2 commits October 21, 2025 09:36
@DavidCEllis
Include comment indicating the issue with VALUE annotations should id…
86da9b8
…eally be resolved.
@DavidCEllis
Use a dunder name instead of a single underscore
d6c680d
@DavidCEllis
Use the dunder name in the tests too
161f3e2
@JelleZijlstra JelleZijlstra added the needs backport to 3.14 bugs and security fixes label Oct 21, 2025
@JelleZijlstra

JelleZijlstra commented Oct 21, 2025

Copy link
Copy Markdown
Member

You'll also need a NEWS entry. Since we're planning to backport to 3.14 to fix the release blocker, I think it's also worth mentioning in What's New as this will be a fairly big change in 3.14.1.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@blurb-it
📜🤖 Added by blurb_it.
e48d6de
@encukou

encukou commented Nov 3, 2025

Copy link
Copy Markdown
Member

Looks like the What's New entry all that's left here. @DavidCEllis, do you want to write one? Or I can do it to help get this in.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DavidCEllis

DavidCEllis commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

I'm happy for you to do it. It's not something I've needed to do before so I don't actually know where it would need to go.

encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
Add What's New entry
a5a53a1
@encukou
encukou requested a review from AA-Turner as a code owner November 3, 2025 15:09
@encukou
Merge in the main branch
add658f
@encukou

encukou commented Nov 3, 2025

Copy link
Copy Markdown
Member

It would go here. For the backport, there'd be a “Notable changes” section at the end, like for 3.13.


@JelleZijlstra ... um, are you sure this is worth it? From an outside point of view this looks like a straightforward regression fix.

DavidCEllis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Nov 7, 2025

Copy link
Copy Markdown
Member

ping @JelleZijlstra

I plan to merge Monday or so if I don't hear back, to give enough time for a backport.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
encukou merged commit 12837c6 into python:main Nov 10, 2025
4 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app

miss-islington-app Bot commented Nov 10, 2025

Copy link
Copy Markdown

Thanks @DavidCEllis for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting review label Nov 10, 2025
@miss-islington-app

miss-islington-app Bot commented Nov 10, 2025

Copy link
Copy Markdown

Sorry, @DavidCEllis and @encukou, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 12837c63635559873a5abddf511d38456d69617b 3.14
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app miss-islington-app Bot assigned encukou Nov 10, 2025
JelleZijlstra pushed a commit to JelleZijlstra/cpython that referenced this pull request Nov 10, 2025
@DavidCEllis @JelleZijlstra
[3.14] pythongh-137530: generate an __annotate__ function for datacla…
9c87484
…sses __init__ (pythonGH-137711)

(cherry picked from commit 12837c6)

Co-authored-by: David Ellis <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 10, 2025

Copy link
Copy Markdown

GH-141352 is a backport of this pull request to the 3.14 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Nov 10, 2025
@JelleZijlstra

JelleZijlstra commented Nov 10, 2025

Copy link
Copy Markdown
Member

Sorry for not responding here earlier, haven't been paying enough attention to GitHub.

um, are you sure this is worth it? From an outside point of view this looks like a straightforward regression fix.

Fair enough, my thinking was that this significantly changes the annotations people will see if they introspect dataclasses init functions. But our What's New documents don't seem to even have sections for changes in bugfix releases recently, so maybe it's not worth it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra added a commit that referenced this pull request Nov 10, 2025
@JelleZijlstra @DavidCEllis
[3.14] gh-137530: generate an __annotate__ function for dataclasses _…
727cdcb
…_init__ (GH-137711) (#141352)

(cherry picked from commit 12837c6)

Co-authored-by: David Ellis <[email protected]>
@encukou

encukou commented Nov 11, 2025

Copy link
Copy Markdown
Member

But our What's New documents don't seem to even have sections for changes in bugfix releases recently, so maybe it's not worth it.

They do, at the end -- for example, https://docs.python.org/3.12/whatsnew/3.12.html#notable-changes-in-3-12-4
It's mainly used for security fixes backported to all active versions, though.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@emontnemery emontnemery mentioned this pull request Dec 3, 2025
Dataclasses without __init__ no longer work in 3.14.1 due to #137711 #142214
Closed
@hugovk hugovk mentioned this pull request Dec 4, 2025
Bug in 3.14.1 Release: Dataclass triggers exception when init=False and slots=True #142251
Closed
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@DavidCEllis @StanFromIreland
pythongh-137530: generate an __annotate__ function for dataclasses __…
33519a4
…init__ (pythonGH-137711)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@encukou encukou

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@DavidCEllis @JelleZijlstra @encukou
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.