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-89687: fix `get_type_hints` with dataclasses `__init__` generation by sobolevn · Pull Request #29158 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Oct 22, 2021 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This is a rather brave idea to manipulate globals, but it looks like it works.
It can backfire is some corner cases, though. But, I am not able to think about any. Ideas? 🙂

https://bugs.python.org/issue45524

CC @AlexWaygood @aidan.b.clark

  • Issue: Cross-module dataclass inheritance breaks get_type_hints #89687

Sorry, something went wrong.

Uh oh!

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

All reactions
@sobolevn
sobolevn requested review from Fidget-Spinner, ericvsmith and gvanrossum as code owners October 22, 2021 12:15
@bedevere-bot bedevere-bot added the awaiting review label Oct 22, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 22, 2021
AlexWaygood
AlexWaygood requested changes Oct 22, 2021
View reviewed changes

@AlexWaygood AlexWaygood left a comment •
edited
Loading

Uh oh!

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

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 like a good fix -- the test script I linked to in the BPO issue passes fine (as do the tests you've added, obviously). Nice job!

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/dataclasses.py Resolved

Uh oh!

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

Comment thread Lib/test/test_typing.py Outdated
Comment on lines 3301 to 3335

@AlexWaygood AlexWaygood Oct 22, 2021

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.

Bikeshedding point, but: would it be better to put these tests in test_dataclasses? For the similar issue with TypedDict, the new tests were put next to the other TypedDict tests. Moreover, given that this is a very specific issue with the way that the dataclasses module generates __init__ functions, it seems like more of a dataclasses bug than a get_type_hints bug.

Sorry, something went wrong.

Uh oh!

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

sobolevn reacted with eyes emoji
All reactions
  • 👀 1 reaction

@sobolevn sobolevn Oct 22, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member 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'm fine with any decision from maintainers 👍
Both cases make sense.

Sorry, something went wrong.

Uh oh!

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

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

@AlexWaygood AlexWaygood Oct 22, 2021

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.

Agreed!

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

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

@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Oct 22, 2021
@sobolevn

sobolevn commented Oct 22, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

I found a corner case: when I define a proxy module, say dataclass_textanno2.py, with this content:

from __future__ import annotations

import dataclasses
from test import dataclass_textanno  # We need to be sure that `Foo` is not in scope


class Custom:
    pass


@dataclasses.dataclass
class Child(dataclass_textanno.Bar):
    custom: Custom


@dataclasses.dataclass(init=False)
class WithFutureInit(Child):
    def __init__(self, foo: dataclass_textanno.Foo, custom: Custom) -> None:
        pass

Then, this test does not pass:

def test_dataclass_from_proxy_module(self):
        from test import dataclass_textanno, dataclass_textanno2
        from dataclasses import dataclass

        @dataclass
        class Default(dataclass_textanno2.Child):
            pass

       self.assertEqual(
                    get_type_hints(Defualt.__init__),
                    {'foo': dataclass_textanno.Foo, 'custom': dataclass_textanno2.Custom,  'return': type(None)},
                )

Output:

======================================================================
ERROR: test_dataclass_from_proxy_module (test.test_typing.GetTypeHintTests) (klass=<class 'test.test_typing.GetTypeHintTests.test_dataclass_from_proxy_module.<locals>.Default'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_typing.py", line 3376, in test_dataclass_from_proxy_module
    get_type_hints(klass.__init__),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/cpython/Lib/typing.py", line 1852, in get_type_hints
    value = _eval_type(value, globalns, localns)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/cpython/Lib/typing.py", line 334, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/cpython/Lib/typing.py", line 700, in _evaluate
    eval(self.__forward_code__, globalns, localns),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'Custom' is not defined

----------------------------------------------------------------------
Ran 1 test in 0.044s

FAILED (errors=1)
test test_typing failed
test_typing failed (1 error)

== Tests result: FAILURE ==

1 test failed:
    test_typing
All reactions

Sorry, something went wrong.

Uh oh!

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

@sobolevn

sobolevn commented Oct 22, 2021

Copy link
Copy Markdown
Member Author

Now it is even more wild. I collect globals from all other mro items especially for __init__ annotations.
This is literally one of the craziest things I've ever done 😆

AlexWaygood reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

AlexWaygood
AlexWaygood reviewed Oct 22, 2021
View reviewed changes
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

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

gvanrossum
gvanrossum reviewed Oct 22, 2021
View reviewed changes
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

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

@sobolevn

sobolevn commented Oct 26, 2021

Copy link
Copy Markdown
Member Author

One more case: same names obviously get overriden by the current logic. I will try something else.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood

AlexWaygood commented Oct 28, 2021

Copy link
Copy Markdown
Member

One more case: same names obviously get overriden by the current logic. I will try something else.

Not sure I understand. What's a failing test case?

All reactions

Sorry, something went wrong.

Uh oh!

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

@sobolevn

sobolevn commented Oct 29, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

@AlexWaygood done! Here's the failing test case: https://github.com/python/cpython/pull/29158/files#diff-f0436842533448f8f02a19081d7c9cc8372b6685756caf32c83dfecebbee68e7R3189-R3219

And here's my new solution: https://github.com/python/cpython/pull/29158/files#diff-44ce2dc1c4922b2f5cf7631d8f86cc569a4c25eb003aaecdc2bc22eb9163d5f5R455-R468

I now manually resolve str types into ForwardRef if they are used for __init__.
This looks like the most logical and easy way to solve our problem.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood

AlexWaygood commented Oct 29, 2021

Copy link
Copy Markdown
Member

@AlexWaygood done! Here's the failing test case: https://github.com/python/cpython/pull/29158/files#diff-f0436842533448f8f02a19081d7c9cc8372b6685756caf32c83dfecebbee68e7R3189-R3219

And here's my new solution: https://github.com/python/cpython/pull/29158/files#diff-44ce2dc1c4922b2f5cf7631d8f86cc569a4c25eb003aaecdc2bc22eb9163d5f5R455-R468

I now manually resolve str types into ForwardRef if they are used for __init__. This looks like the most logical and easy way to solve our problem.

Ahh, I understand -- thanks!

All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood
AlexWaygood reviewed Oct 29, 2021
View reviewed changes
Comment thread Lib/dataclasses.py Resolved

Uh oh!

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

Comment thread Lib/test/dataclass_textanno2.py Resolved

Uh oh!

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

Comment thread Lib/test/dataclass_textanno2.py Resolved

Uh oh!

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

@sobolevn

sobolevn commented Oct 29, 2021

Copy link
Copy Markdown
Member Author

Thanks! Done 👍

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

@github-actions

github-actions Bot commented Nov 29, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

AlexWaygood and chirizxc reacted with confused emoji
All reactions
  • 😕 2 reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Nov 29, 2021
sobolevn and others added 9 commits November 29, 2021 11:51
@sobolevn
bpo-45524: fix get_type_hints with dataclasses __init__ generation
acd9750
@sobolevn @AlexWaygood
Update Lib/dataclasses.py
e057993
Co-authored-by: Alex Waygood <[email protected]>
@sobolevn @AlexWaygood
Update Lib/test/test_typing.py
35015d0
Co-authored-by: Alex Waygood <[email protected]>
@sobolevn
Ever more wild globals manipulation
a60d850
@sobolevn
Uncomment tests
304a658
@sobolevn
Move tests from test_typing to test_dataclasses
720924e
@sobolevn
Manually resolve string types into ForwardRef in __init__ of a da…
97a3496
…taclass
@sobolevn @AlexWaygood
Update Lib/test/dataclass_textanno2.py
8a32156
Co-authored-by: Alex Waygood <[email protected]>
@sobolevn
Address review
df5fda9
@sobolevn
sobolevn force-pushed the issue-45524 branch from 7205490 to df5fda9 Compare November 29, 2021 08:54
@sobolevn

sobolevn commented Nov 29, 2021

Copy link
Copy Markdown
Member Author

Rebased 🙂

Friendly ping to @ericvsmith and @Fidget-Spinner

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Nov 30, 2021
@AlexWaygood AlexWaygood mentioned this pull request Jan 12, 2022
bpo-46333: Honor module parameter in ForwardRef #30536
Merged
JelleZijlstra
JelleZijlstra approved these changes Mar 10, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Mar 10, 2022
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@sobolevn
Merge branch 'main' into issue-45524
3ad08f5
@sobolevn sobolevn closed this Aug 28, 2022
@sobolevn sobolevn reopened this Aug 28, 2022
@ericvsmith

ericvsmith commented Oct 4, 2022

Copy link
Copy Markdown
Member

I'm putting this off until the SC makes a decision on PEP 649.

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

@ericvsmith ericvsmith self-assigned this Oct 4, 2022
@hauntsaninja hauntsaninja added the DO-NOT-MERGE label Jan 7, 2023
@aidanbclark aidanbclark mannequin mentioned this pull request Aug 16, 2024
Cross-module dataclass inheritance breaks get_type_hints #89687
Open
@encukou
Merge in the main branch
fbb92cb
@picnixz picnixz changed the title bpo-45524: fix get_type_hints with dataclasses __init__ generation gh-89687: fix get_type_hints with dataclasses __init__ generation Feb 24, 2025
@Tishka17

Tishka17 commented Jul 28, 2025

Copy link
Copy Markdown

PEP 649 is accepted and implemented in 3.14, so what should be done with this PR?

All reactions

Sorry, something went wrong.

Uh oh!

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

@sobolevn

sobolevn commented Jul 28, 2025

Copy link
Copy Markdown
Member Author

I lost context of this, please recreate :)

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

@sobolevn sobolevn closed this Jul 28, 2025
@Tishka17 Tishka17 mentioned this pull request Jul 28, 2025
gh-89687: fix get_type_hints with dataclasses __init__ generation #137168
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum left review comments
@AlexWaygood AlexWaygood AlexWaygood requested changes
@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner
@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner

Uh oh!

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

Assignees

@ericvsmith ericvsmith

Labels

awaiting merge DO-NOT-MERGE

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.

11 participants

@sobolevn @AlexWaygood @ericvsmith @Tishka17 @JelleZijlstra @gvanrossum @hauntsaninja @the-knights-who-say-ni @ezio-melotti @bedevere-bot @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.