Skip to content

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

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
Merged
kumaraditya303 merged 4 commits into from
Nov 5, 2024
Merged

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

kumaraditya303 merged 4 commits into from
Nov 5, 2024

Conversation

@tungol
Copy link
Contributor

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

@tungoltungol requested a review from gpshead as a code ownerNovember 5, 2024 01:02
@ghost
Copy link

ghost commented Nov 5, 2024•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-appbedevere-appbot mentioned this pull request Nov 5, 2024
Closed
@tungoltungol mentioned this pull request Nov 5, 2024
Closed
ZeroIntensity
ZeroIntensity reviewed Nov 5, 2024
View reviewed changes
Copy link
Member

@ZeroIntensityZeroIntensity 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

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

@ZeroIntensityZeroIntensity 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

@AlexWaygoodAlexWaygood 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!

@kumaraditya303kumaraditya303 merged commit 78842e4 into python:mainNov 5, 2024
@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 5, 2024

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

kumaraditya303 commented Nov 5, 2024

I am fine with backporting this

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

miss-islington-appbot commented Nov 5, 2024

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

miss-islington-appbot commented Nov 5, 2024

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

miss-islington-appbot commented Nov 5, 2024

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

miss-islington-appbot commented Nov 5, 2024

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)
@AlexWaygoodAlexWaygood mentioned this pull request Nov 5, 2024
Merged
@bedevere-app
Copy link

bedevere-appbot commented Nov 5, 2024

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

@bedevere-appbedevere-appbot 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)
@AlexWaygoodAlexWaygood mentioned this pull request Nov 5, 2024
Merged
@bedevere-app
Copy link

bedevere-appbot commented Nov 5, 2024

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

@bedevere-appbedevere-appbot removed the needs backport to 3.12 only 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 <[email protected]>
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 <[email protected]>
@tungoltungol deleted the multiprocessing-proxies branch November 5, 2024 18:10
@AlexWaygoodAlexWaygood mentioned this pull request Nov 6, 2024
Merged
@sobolevnsobolevn 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.
@AlexWaygoodAlexWaygood 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.
@AlexWaygoodAlexWaygood 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensityZeroIntensityZeroIntensity approved these changes

@kumaraditya303kumaraditya303kumaraditya303 approved these changes

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

@gpsheadgpsheadAwaiting requested review from gpsheadgpshead is a code owner

Assignees

@AlexWaygoodAlexWaygood

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