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 — Rename typing._collect_parameters by JelleZijlstra · Pull Request #118900 · python/cpython

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented May 10, 2024

Copy link
Copy Markdown
Member

function. Unfortunately, released versions of typing_extensions
monkeypatch this function without the extra parameter, which makes
it so things break badly if current main is used with typing_extensions.

% ~/py/cpython/python.exe test_typing_extensions.py
Traceback (most recent call last):
  File "/Users/jelle/py/typing_extensions/src/test_typing_extensions.py", line 42, in <module>
    from _typed_dict_test_helper import Foo, FooGeneric, VeryAnnotated
  File "/Users/jelle/py/typing_extensions/src/_typed_dict_test_helper.py", line 17, in <module>
    class FooGeneric(TypedDict, Generic[T]):
                                ~~~~~~~^^^
  File "/Users/jelle/py/cpython/Lib/typing.py", line 431, in inner
    return func(*args, **kwds)
  File "/Users/jelle/py/cpython/Lib/typing.py", line 1243, in _generic_class_getitem
    return _GenericAlias(cls, args)
  File "/Users/jelle/py/cpython/Lib/typing.py", line 1420, in __init__
    self.__parameters__ = _collect_parameters(
                          ~~~~~~~~~~~~~~~~~~~^
        args,
        ^^^^^
        enforce_default_ordering=enforce_default_ordering,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
TypeError: _collect_parameters() got an unexpected keyword argument 'enforce_default_ordering'

Fortunately, the monkeypatching is not needed on Python 3.13, because CPython
now implements PEP 696. By renaming the function, we prevent the monkeypatch
from breaking typing.py internals.

Sorry, something went wrong.

Uh oh!

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

All reactions
@JelleZijlstra
Rename typing._collect_parameters
1020121
function. Unfortunately, released versions of typing_extensions
monkeypatch this function without the extra parameter, which makes
it so things break badly if current main is used with typing_extensions.

```
% ~/py/cpython/python.exe test_typing_extensions.py
Traceback (most recent call last):
  File "/Users/jelle/py/typing_extensions/src/test_typing_extensions.py", line 42, in <module>
    from _typed_dict_test_helper import Foo, FooGeneric, VeryAnnotated
  File "/Users/jelle/py/typing_extensions/src/_typed_dict_test_helper.py", line 17, in <module>
    class FooGeneric(TypedDict, Generic[T]):
                                ~~~~~~~^^^
  File "/Users/jelle/py/cpython/Lib/typing.py", line 431, in inner
    return func(*args, **kwds)
  File "/Users/jelle/py/cpython/Lib/typing.py", line 1243, in _generic_class_getitem
    return _GenericAlias(cls, args)
  File "/Users/jelle/py/cpython/Lib/typing.py", line 1420, in __init__
    self.__parameters__ = _collect_parameters(
                          ~~~~~~~~~~~~~~~~~~~^
        args,
        ^^^^^
        enforce_default_ordering=enforce_default_ordering,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
TypeError: _collect_parameters() got an unexpected keyword argument 'enforce_default_ordering'
```

Fortunately, the monkeypatching is not needed on Python 3.13, because CPython
now implements PEP 696. By renaming the function, we prevent the monkeypatch
from breaking typing.py internals.
@JelleZijlstra JelleZijlstra added skip issue skip news labels May 10, 2024
@JelleZijlstra
JelleZijlstra requested a review from AlexWaygood as a code owner May 10, 2024 13:28
@bedevere-app bedevere-app Bot added the awaiting core review label May 10, 2024
@JelleZijlstra JelleZijlstra added topic-typing needs backport to 3.13 only security fixes labels May 10, 2024
@AlexWaygood

AlexWaygood commented May 10, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

There are some uses of this function in the wild, which this change would break... https://github.com/wyfo/apischema/blob/d48cc010c417e9c49db25b7a8683621b3c305b44/apischema/typing.py#L59-L62

And we could also fix the test failure over at typing_extensions with this diff:

--- a/src/typing_extensions.py
+++ b/src/typing_extensions.py
@@ -2836,7 +2836,8 @@ else:
 
         return tuple(parameters)
 
-    typing._collect_parameters = _collect_parameters
+    if not _PEP_696_IMPLEMENTED:
+        typing._collect_parameters = _collect_parameters

It's true that making a change in CPython rather than typing_extensions has the advantage that already-released versions of typing_extensions are fixed as well to-be-released versions of typing_extensions. But I think already-released versions of typing_extensions are badly broken anyway on Python 3.13, due to the error pointed out in python/typing_extensions#377 (comment)

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood AlexWaygood mentioned this pull request May 10, 2024
Remove some unnecessary monkeypatching on Python 3.13+ python/typing_extensions#379
Merged
@JelleZijlstra

JelleZijlstra commented May 10, 2024

Copy link
Copy Markdown
Member Author

But I think already-released versions of typing_extensions are badly broken anyway on Python 3.13, due to the error pointed out in python/typing_extensions#377 (comment)

I think that error shows up only if you use typing_extensions.TypeVar though, while the error this PR fixes breaks any use of Generic after typing_extensions is imported, which feels much worse.

There are some uses of this function in the wild, which this change would break... https://github.com/wyfo/apischema/blob/d48cc010c417e9c49db25b7a8683621b3c305b44/apischema/typing.py#L59-L62

I think we could address this by keeping an alias _collect_parameters = _collect_type_parameters (or a wrapper function that raises DeprecationWarning).

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.

@AlexWaygood

AlexWaygood commented May 10, 2024

Copy link
Copy Markdown
Member

Okay, sounds reasonable.

I think we could address this by keeping an alias _collect_parameters = _collect_type_parameters (or a wrapper function that raises DeprecationWarning).

Or we could do this in the __getattr__ method down at the bottom? (With do_the_deprecation_warning() replaced by the appropriate logic to do a deprecation warning)

--- a/Lib/typing.py
+++ b/Lib/typing.py
@@ -3770,6 +3770,9 @@ def __getattr__(attr):
     elif attr in {"ContextManager", "AsyncContextManager"}:
         import contextlib
         obj = _alias(getattr(contextlib, f"Abstract{attr}"), 2, name=attr, defaults=(bool | None,))
+    elif attr == "_collect_parameters":
+        do_the_deprecation_warning()
+        return _collect_type_parameters
     else:
         raise AttributeError(f"module {__name__!r} has no attribute {attr!r}")
     globals()[attr] = obj
JelleZijlstra 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.

JelleZijlstra added 3 commits May 10, 2024 08:45
@JelleZijlstra
Readd as deprecated
d7a7c0c
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into collectparams
b472e26
@JelleZijlstra
Better test
0afb4a7
AlexWaygood
AlexWaygood reviewed May 10, 2024
View reviewed changes
Comment thread Lib/typing.py Resolved

Uh oh!

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

@JelleZijlstra
fix stacklevel
b022ddc
AlexWaygood
AlexWaygood approved these changes May 10, 2024
View reviewed changes

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

Thanks!

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-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 10, 2024
@JelleZijlstra @AlexWaygood
Update Lib/test/test_typing.py
388d56b
Co-authored-by: Alex Waygood <[email protected]>
@JelleZijlstra
JelleZijlstra enabled auto-merge (squash) May 10, 2024 16:21
@JelleZijlstra
JelleZijlstra merged commit ec9d12b into python:main May 10, 2024
@miss-islington-app

miss-islington-app Bot commented May 10, 2024

Copy link
Copy Markdown

Thanks @JelleZijlstra for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

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 merge label May 10, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 10, 2024
@JelleZijlstra @miss-islington
Rename typing._collect_parameters (pythonGH-118900)
34def64
Unfortunately, released versions of typing_extensions
monkeypatch this function without the extra parameter, which makes
it so things break badly if current main is used with typing_extensions.

Fortunately, the monkeypatching is not needed on Python 3.13, because CPython
now implements PEP 696. By renaming the function, we prevent the monkeypatch
from breaking typing.py internals.

We keep the old name (raising a DeprecationWarning) to help other external users who call it.
(cherry picked from commit ec9d12b)

Co-authored-by: Jelle Zijlstra <[email protected]>
@bedevere-app

bedevere-app Bot commented May 10, 2024

Copy link
Copy Markdown

GH-118917 is a backport of this pull request to the 3.13 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.13 only security fixes label May 10, 2024
@JelleZijlstra
JelleZijlstra deleted the collectparams branch May 10, 2024 17:08
AlexWaygood pushed a commit that referenced this pull request May 10, 2024
@miss-islington @JelleZijlstra
[3.13] Rename typing._collect_parameters (GH-118900) (#118917)
b3074f0
Co-authored-by: Jelle Zijlstra <[email protected]>
@bedevere-bot

bedevere-bot commented May 10, 2024

Copy link
Copy Markdown

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

Hi! The buildbot AMD64 Ubuntu NoGIL 3.13 has failed when building commit b3074f0.

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/all/#builders/1422/builds/9) and take a look at the build logs.
  4. Check if the failure is related to this commit (b3074f0) 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/all/#builders/1422/builds/9

Failed tests:

  • test_eintr
  • test_math

Failed subtests:

  • test_flock - main.FNTLEINTRTest.test_flock
  • test_all - test.test_eintr.EINTRTests.test_all
  • test_wait_integer - test.test_multiprocessing_spawn.test_misc.TestWait.test_wait_integer
  • test_map_timeout - test.test_concurrent_futures.test_process_pool.ProcessPoolSpawnProcessPoolExecutorTest.test_map_timeout
  • test_lockf - main.FNTLEINTRTest.test_lockf

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

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 532, in test_lockf
    self._lock(fcntl.lockf, "lockf")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.5 sec


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/test_eintr.py", line 17, in test_all
    script_helper.run_test_script(script)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/support/script_helper.py", line 316, in run_test_script
    raise AssertionError(f"{name} failed")
AssertionError: script _test_eintr.py failed


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_multiprocessing.py", line 5176, in test_wait_integer
    self.assertLess(delta, expected + 2)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
AssertionError: 5.133446088992059 not less than 5


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 535, in test_flock
    self._lock(fcntl.flock, "flock")
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/_test_eintr.py", line 517, in _lock
    raise Exception("failed to sync child in %.1f sec" % dt)
Exception: failed to sync child in 300.5 sec


Traceback (test.test_zipimport.CompressedZipImportTestCase.testTraceback) ... ok


Traceback (test.test_zipimport.UncompressedZipImportTestCase.testTraceback) ... ok


Traceback (most recent call last):
  File "/home/ubuntu/buildarea/3.13.itamaro-ubuntu-aws.nogil/build/Lib/test/test_concurrent_futures/executor.py", line 71, in test_map_timeout
    self.assertEqual([None, None], results)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [None, None] != []
All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra JelleZijlstra mentioned this pull request May 28, 2024
backport a9a74da 3.13 #119642
Closed
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@JelleZijlstra @estyxx
Rename typing._collect_parameters (python#118900)
e43acde
Unfortunately, released versions of typing_extensions
monkeypatch this function without the extra parameter, which makes
it so things break badly if current main is used with typing_extensions.

Fortunately, the monkeypatching is not needed on Python 3.13, because CPython
now implements PEP 696. By renaming the function, we prevent the monkeypatch
from breaking typing.py internals.

We keep the old name (raising a DeprecationWarning) to help other external users who call it.
@brianschubert brianschubert mentioned this pull request Sep 1, 2025
Fix how type parameters are collected from parametrized Protocol bases python/typing_extensions#667
Open
@coderabbitai coderabbitai Bot mentioned this pull request Jul 27, 2026
stdlib: expand compatibility foundations pyre-lang/pyre#845
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip issue skip news topic-typing

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

@JelleZijlstra @AlexWaygood @bedevere-bot
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 01402520 — 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.