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-132805: annotationlib: Fix handling of non-constant values in FORWARDREF by JelleZijlstra · Pull Request #132812 · python/cpython

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Apr 22, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Fixes #132805. Closes #132806. cc @DavidCEllis.

  • Issue: [3.14] annotationlib - Union '|' syntax and typing.Union[...] generate different forward references. #132805

Sorry, something went wrong.

Uh oh!

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

All reactions
DavidCEllis and others added 8 commits April 22, 2025 16:53
@DavidCEllis
Add a test for union forward references
fba5efa
@DavidCEllis
Make stringifiers create unions if create_unions is True
d810953
@DavidCEllis
modify broken test, move test to forwardref format group
1908a4a
@DavidCEllis
Apparently trim trailing whitespace was turned off
97abdc7
@JelleZijlstra
Alternative approach
dd36a62
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into fix-union-forwardref
20f3dac
@JelleZijlstra
fixes
8266a5e
@JelleZijlstra
blurb
1c629dc
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 22, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 22, 2025
[3.14] annotationlib - Union '|' syntax and typing.Union[...] generate different forward references. #132805
Closed
DavidCEllis
DavidCEllis reviewed Apr 22, 2025
View reviewed changes
Comment thread Lib/annotationlib.py Resolved

Uh oh!

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

JelleZijlstra added 2 commits April 22, 2025 14:07
@JelleZijlstra
Handle displays
3d7fec8
@JelleZijlstra
remove create_unions
ec4dd0e
TeamSpen210
TeamSpen210 reviewed Apr 22, 2025
View reviewed changes
Comment thread Lib/annotationlib.py Resolved

Uh oh!

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

@JelleZijlstra
prefer empty dict
6e31c69
@DavidCEllis

DavidCEllis commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

This PR does mean that partial evaluations of the different ways of writing unions aren't quite equivalent. A union written with | with an element that can't be evaluated gives no information about the types, while a union written with Union[...] gives partial information1.

  • str | undefined -> ForwardRef('__annotationlib_name__1 | undefined', ...)
  • Union[str, undefined] -> str | ForwardRef('undefined', ...)

This does make me think that perhaps the previous change to make a | b into Union[a, b] might still be worth doing? Arguably a and b in this case are forward references and | between forward references creates a union.

If not, then perhaps the __extra_names__ should be more visible to help distinguish between otherwise similar looking forward references with different contents.

Example:

from annotationlib import get_annotations, Format

class Ex1:
    a: str | undefined
    b: str | int | undefined

for k, v in get_annotations(Ex1, format=Format.FORWARDREF).items():
    print(f"{k}: {v}")
a: ForwardRef('__annotationlib_name_1__ | undefined', is_class=True, owner=<class '__main__.Ex1'>)
b: ForwardRef('__annotationlib_name_2__ | undefined', is_class=True, owner=<class '__main__.Ex1'>)

Footnotes

  1. Side note: is there a helper function somewhere to try to evaluate any forwardrefs contained in a union or generic alias? ↩

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented Apr 23, 2025

Copy link
Copy Markdown
Member Author

I sort of think that's correct: if we don't know what A and B are, we don't know what A | B will evaluate to. It might be a union, but it might be something else too if A happens to implement __or__ some other way.

We could perhaps hide the __annotation_name_1__ names in the repr() of ForwardRefs, though, do you think that would be better?

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra
Do not use special names in STRING format
525a4e4
@DavidCEllis

DavidCEllis commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

I sort of think that's correct: if we don't know what A and B are, we don't know what A | B will evaluate to. It might be a union, but it might be something else too if A happens to implement __or__ some other way.

My logic is more along the lines of - if a name can't be resolved it becomes a forward ref, anything that uses | with a forwardref becomes a union1 thus it makes sense by that logic for this to become a union.

The other reasoning is that a runtime checker could use the partial information to know that an argument is valid in the case where you get a union, if the type is one of the elements that does resolve, but couldn't do so in the case where the entire statement is a forwardref. I'm not sure how common this case would be though.

We could perhaps hide the __annotation_name_1__ names in the repr() of ForwardRefs, though, do you think that would be better?

I'm not sure about this one, I know the repr already isn't exactly complete though with all the hidden internals.

Footnotes

  1. Provided it doesn't have an __or__ that does something else with a forwardref. ↩

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented Apr 24, 2025

Copy link
Copy Markdown
Member Author

It still feels wrong to me to special-case just __or__; your suggestion would mean this operator works differently than all the others.

I'm actually not convinced ForwardRef.__or__ should exist at all, as ForwardRefs shouldn't be created in a place where users create Unions out of them. It was added in https://bugs.python.org/issue45489 without much discussion. Removing it would be a compatibility issue at this point though, so I'm not proposing it.

All reactions

Sorry, something went wrong.

Uh oh!

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

@DavidCEllis

DavidCEllis commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

That makes sense, it just bugs me that this means a | b is not the same as Union[a, b] if you're using type hints in this context (and one happens to be a forwardref). Unfortunately tools like pyupgrade will automatically convert Union syntax to | and now in this case they're not equivalent.

All reactions

Sorry, something went wrong.

Uh oh!

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

JelleZijlstra added 3 commits May 4, 2025 07:25
@JelleZijlstra
Merge branch 'main' into extra-names
0a75e5f
@JelleZijlstra
Merge branch 'main' into extra-names
7dec46b
@JelleZijlstra
fix bad merge
22a9691
@JelleZijlstra
JelleZijlstra merged commit c8f233c into python:main May 4, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label May 4, 2025
@bedevere-bot

bedevere-bot commented May 4, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x (tier-3) has failed when building commit c8f233c.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/3537) and take a look at the build logs.
  4. Check if the failure is related to this commit (c8f233c) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/3537

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/99D824FA-F7CC-45B8-8E9C-C9B3DCBBA39B/data/Containers/Bundle/Application/9DCD2A29-C3D4-4FA7-B324-25A9D853418C/iOSTestbed.app/python/lib/python3.14/threading.py", line 1079, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/99D824FA-F7CC-45B8-8E9C-C9B3DCBBA39B/data/Containers/Bundle/Application/9DCD2A29-C3D4-4FA7-B324-25A9D853418C/iOSTestbed.app/python/lib/python3.14/threading.py", line 1021, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/99D824FA-F7CC-45B8-8E9C-C9B3DCBBA39B/data/Containers/Bundle/Application/9DCD2A29-C3D4-4FA7-B324-25A9D853418C/iOSTestbed.app/python/lib/python3.14/test/test_interpreters/test_stress.py", line 30, in task
    interp = interpreters.create()
  File "/Users/buildbot/Library/Developer/XCTestDevices/99D824FA-F7CC-45B8-8E9C-C9B3DCBBA39B/data/Containers/Bundle/Application/9DCD2A29-C3D4-4FA7-B324-25A9D853418C/iOSTestbed.app/python/lib/python3.14/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k
All reactions

Sorry, something went wrong.

Uh oh!

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

diegorusso added a commit to diegorusso/cpython that referenced this pull request May 4, 2025
@diegorusso
Merge remote-tracking branch 'origin/main' into call-len
c03babc
* origin/main: (111 commits)
  pythongh-91048: Add filename and line number to external inspection routines (pythonGH-133385)
  pythongh-131178: Add tests for `ast` command-line interface (python#133329)
  Regenerate pcbuild.sln in Visual Studio 2022 (python#133394)
  pythongh-133042: disable HACL* HMAC on Emscripten (python#133064)
  pythongh-133351: Fix remote PDB's multi-line block tab completion (python#133387)
  pythongh-109700: Improve stress tests for interpreter creation (pythonGH-109946)
  pythongh-81793: Skip tests for os.link() to symlink on Android (pythonGH-133388)
  pythongh-126835: Rename `ast_opt.c` to `ast_preprocess.c` and related stuff after moving const folding to the peephole optimizier (python#131830)
  pythongh-91048: Relax test_async_global_awaited_by to fix flakyness (python#133368)
  pythongh-132457: make staticmethod and classmethod generic (python#132460)
  pythongh-132805: annotationlib: Fix handling of non-constant values in FORWARDREF (python#132812)
  pythongh-132426: Add get_annotate_from_class_namespace replacing get_annotate_function (python#132490)
  pythongh-81793: Always call linkat() from os.link(), if available (pythonGH-132517)
  pythongh-122559: Synchronize C and Python implementation of the io module about pickling (pythonGH-122628)
  pythongh-69605: Add PyREPL import autocomplete feature to 'What's New' (python#133358)
  bpo-44172: Keep reference to original window in curses subwindow objects (pythonGH-26226)
  pythonGH-133231: Changes to executor management to support proposed `sys._jit` module (pythonGH-133287)
  pythongh-133363: Fix Cmd completion for lines beginning with `! ` (python#133364)
  pythongh-132983: Introduce `_zstd` bindings module (pythonGH-133027)
  pythonGH-91048: Add utils for printing the call stack for asyncio tasks (python#133284)
  ...
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@JelleZijlstra @DavidCEllis @Pranjal095
pythongh-132805: annotationlib: Fix handling of non-constant values i…
73a12cb
…n FORWARDREF (python#132812)

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

Reviewers

@DavidCEllis DavidCEllis DavidCEllis left review comments
+1 more reviewer
@TeamSpen210 TeamSpen210 TeamSpen210 left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[3.14] annotationlib - Union '|' syntax and typing.Union[...] generate different forward references.

Uh oh!

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

4 participants

@JelleZijlstra @DavidCEllis @bedevere-bot @TeamSpen210
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.