Skip to content

Conversation

@dr-carlos
Copy link
Contributor

@dr-carlos dr-carlos commented Sep 3, 2025

Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

looks good

Co-authored-by: sobolevn <[email protected]>
@python-cla-bot
Copy link

python-cla-bot bot commented Sep 3, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

…fined generic tests

Removed global overriding builtin test as it seemingly wasn't compatible with `get_annotations()`.
@dr-carlos
Copy link
Contributor Author

Note that this PR also inherently fixes builtins overriding globals in this particular code path. I was previously unable to come up with a test for this without using the undocumented ForwardRef() constructor. I have since done some thinking, and maybe I am missing something obvious, but all I could come up with was an (extremely contrived) test:

from annotationlib import Format, get_annotations

class C:
	y: alias[memoryview, undef]

mv = __builtins__["memoryview"]
del __builtins__["memoryview"]
fwdref = get_annotations(C, format=Format.FORWARDREF)["y"]

__builtins__["memoryview"] = mv
generic = fwdref.evaluate(
	format=Format.FORWARDREF,
	globals={"memoryview": int},
	locals={"alias": dict}
)
self.assertNotIsInstance(generic, ForwardRef)
self.assertIs(generic.__origin__, dict)
self.assertEqual(len(generic.__args__), 2)
self.assertIs(generic.__args__[0], int)
self.assertIsInstance(generic.__args__[1], ForwardRef)

It feels very unlikely this would happen in real code, and I'm not sure if it's a good idea to test given that "Since this is an implementation detail, it may not be used by alternate implementations of Python." (https://docs.python.org/3/library/builtins.html), and given that __builtins__ seems to be either a module object or a dictionary depending on where you run the code.

@JelleZijlstra
Copy link
Member

Good point, I think it's fine to skip the test in that case.

Technical issue: your two PRs #138430 and #138075 will conflict with each other, and I won't be able to backport them to 3.14 until after 3.14.0 final comes out. So I'm planning to leave them open for now and land them once 3.14.0 final is out so I can sort out the merge conflicts and merge the backports without having to disrupt the RC process.

@dr-carlos
Copy link
Contributor Author

Technical issue: your two PRs #138430 and #138075 will conflict with each other, and I won't be able to backport them to 3.14 until after 3.14.0 final comes out. So I'm planning to leave them open for now and land them once 3.14.0 final is out so I can sort out the merge conflicts and merge the backports without having to disrupt the RC process.

@JelleZijlstra Just a quick reminder of these PRs - any updates now that 3.14.0 final is out?

@JelleZijlstra JelleZijlstra merged commit e66f87c into python:main Nov 2, 2025
49 checks passed
@JelleZijlstra JelleZijlstra added the needs backport to 3.14 bugs and security fixes label Nov 2, 2025
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 2, 2025
…defined params in `ref.evaluate(format=Format.FORWARDREF)` (pythonGH-138430)

(cherry picked from commit e66f87c)

Co-authored-by: dr-carlos <[email protected]>
Co-authored-by: sobolevn <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 2, 2025

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

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Nov 2, 2025
JelleZijlstra pushed a commit that referenced this pull request Nov 2, 2025
…ndefined params in `ref.evaluate(format=Format.FORWARDREF)` (GH-138430) (#140927)

gh-138425: Correctly partially evaluate global generics with undefined params in `ref.evaluate(format=Format.FORWARDREF)` (GH-138430)
(cherry picked from commit e66f87c)

Co-authored-by: dr-carlos <[email protected]>
Co-authored-by: sobolevn <[email protected]>
@dr-carlos
Copy link
Contributor Author

Thanks for the merges @JelleZijlstra!

@dr-carlos dr-carlos deleted the annolib-globals branch November 3, 2025 01:23
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
…defined params in `ref.evaluate(format=Format.FORWARDREF)` (python#138430)

Co-authored-by: sobolevn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants