Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-126417: Register multiprocessing proxy types to an appropriate collections.abc class #126419

Merged
merged 4 commits into from
Nov 5, 2024
Merged

gh-126417: Register multiprocessing proxy types to an appropriate collections.abc class #126419

merged 4 commits into from
Nov 5, 2024

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Nov 5, 2024 •
edited by bedevere-app bot
Loading

This MR registers BaseListProxy and _BaseDictProxy in multiprocessing.managers to an appropriate ABC from collections.abc. This improves their consistency with the types they proxy.

While I was here, I also updated the name of _BaseDictProxy so it's consistent with the variable that the type is assigned to. I think that kind of naming consistency is good, but it's less important and I can take that change out if it's not appropriate to include here.

@tungol tungol requested a review from gpshead as a code owner November 5, 2024 01:02
@cpython-cla-bot CPython CLA Bot
Copy link

cpython-cla-bot bot commented Nov 5, 2024 •
edited
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot mentioned this pull request Nov 5, 2024
Closed
@tungol tungol mentioned this pull request Nov 5, 2024
Closed
ZeroIntensity
ZeroIntensity reviewed Nov 5, 2024
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

This needs a test--please add one making sure that isinstance works on these to Lib/test/_test_multiprocessing.py

tungol and others added 2 commits November 4, 2024 19:04
@tungol @ZeroIntensity
Update Misc/NEWS.d/next/Library/2024-11-04-16-40-02.gh-issue-126417.O…
d523fd1 
…WPqn0.rst

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@tungol
add tests
a2b7fd6
ZeroIntensity
ZeroIntensity approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

LGTM

AlexWaygood
AlexWaygood approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thank you! This LGTM. I'll leave it a few days before merging in case Greg or anyone else wants to chime in — please ping me if it's been a week and this is still open!

@kumaraditya303 kumaraditya303 merged commit 78842e4 into python:main Nov 5, 2024
44 checks passed
@AlexWaygood
Copy link
Member

Thoughts from the other reviewers on whether this should be backported? It feels borderline between a feature and a bugfix to me... I lean towards backporting? It think it's quite low-risk.

@kumaraditya303
Copy link
Contributor

I am fine with backporting this

@AlexWaygood AlexWaygood added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Nov 5, 2024
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

Sorry, @tungol and @kumaraditya303, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 78842e4a98994a218a93992a2a1e3ca3eaa28e79 3.13

@miss-islington-app
Copy link

Sorry, @tungol and @kumaraditya303, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 78842e4a98994a218a93992a2a1e3ca3eaa28e79 3.12

AlexWaygood pushed a commit to AlexWaygood/cpython that referenced this pull request Nov 5, 2024
@tungol @AlexWaygood
[3.13] pythongh-126417: Register multiprocessing proxy types to an ap…
1a6c61a 
…propriate collections.abc class (python#126419)

(cherry-picked from commit 78842e4)
@AlexWaygood AlexWaygood mentioned this pull request Nov 5, 2024
Merged
@bedevere-app
Copy link

bedevere-app bot commented Nov 5, 2024

GH-126435 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 5, 2024
AlexWaygood pushed a commit to AlexWaygood/cpython that referenced this pull request Nov 5, 2024
@tungol @AlexWaygood
[3.12] pythongh-126417: Register multiprocessing proxy types to an ap…
1b557b0 
…propriate collections.abc class (python#126419)

(cherry-picked from commit 78842e4)
@AlexWaygood AlexWaygood mentioned this pull request Nov 5, 2024
Merged
@bedevere-app
Copy link

bedevere-app bot commented Nov 5, 2024

GH-126436 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Nov 5, 2024
AlexWaygood added a commit that referenced this pull request Nov 5, 2024
@AlexWaygood @tungol
[3.12] gh-126417: Register multiprocessing proxy types to an appropri…
47d48b6 
…ate collections.abc class (#126419) (#126436)

Co-authored-by: Stephen Morton <github@tungol.org>
AlexWaygood added a commit that referenced this pull request Nov 5, 2024
@AlexWaygood @tungol
[3.13] gh-126417: Register multiprocessing proxy types to an appropri…
c1e708a 
…ate collections.abc class (#126419) (#126435)

Co-authored-by: Stephen Morton <github@tungol.org>
@tungol tungol deleted the multiprocessing-proxies branch November 5, 2024 18:10
@AlexWaygood AlexWaygood mentioned this pull request Nov 6, 2024
Merged
@sobolevn sobolevn mentioned this pull request Nov 7, 2024
Closed
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Nov 12, 2024
@AlexWaygood
Revert "[3.13] pythongh-126417: Register multiprocessing proxy types …
bf7cb54 
…to an appropriate collections.abc class (python#126419) (python#126435)"

This reverts commit c1e708a.
@AlexWaygood AlexWaygood mentioned this pull request Nov 12, 2024
Merged
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Nov 12, 2024
@AlexWaygood
Revert "[3.12] pythongh-126417: Register multiprocessing proxy types …
6514a09 
…to an appropriate collections.abc class (python#126419) (python#126436)"

This reverts commit 47d48b6.
@AlexWaygood AlexWaygood mentioned this pull request Nov 12, 2024
Merged
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
@tungol @picnixz
pythongh-126417: Register multiprocessing proxy types to an appropria…
ed57168 
…te collections.abc class (python#126419)
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@tungol @ebonnal
pythongh-126417: Register multiprocessing proxy types to an appropria…
ae487ce 
…te collections.abc class (python#126419)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner

Assignees

@AlexWaygood AlexWaygood

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tungol @AlexWaygood @kumaraditya303 @ZeroIntensity