Skip to content

bpo-45081: Fix __init__ method generation when inheriting from Protocol - #28121

Merged
ambv merged 5 commits into
python:mainfrom
uriyyo:fix-issue-45081
Sep 2, 2021
Merged

bpo-45081: Fix __init__ method generation when inheriting from Protocol#28121
ambv merged 5 commits into
python:mainfrom
uriyyo:fix-issue-45081

Conversation

@uriyyo

@uriyyouriyyo commented Sep 2, 2021•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@uriyyo

uriyyo commented Sep 2, 2021

Copy link
Copy Markdown
MemberAuthor

@Fidget-Spinner Could you please verify that we need backport those changes to 3.9 and 3.10 versions?

ericvsmith
ericvsmith reviewed Sep 2, 2021
View reviewed changes
Comment threadLib/dataclasses.py Outdated
Fidget-Spinner
Fidget-Spinner reviewed Sep 2, 2021
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@uriyyo if I understand the original issue correctly, you're right that we need to backport this to fix that issue's backports.

I'm deferring this to @serhiy-storchaka or @ambv because I'm not too familiar with the previous patch that caused this regression. Sorry.

Comment threadLib/dataclasses.py Outdated
@uriyyo

uriyyo commented Sep 2, 2021

Copy link
Copy Markdown
MemberAuthor

@Fidget-Spinner Could you please review this PR? Looks like I found way how to fix this issue without changing dataclasses module.

Fidget-Spinner
Fidget-Spinner reviewed Sep 2, 2021
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After researching this issue (and the previous patch), this LGTM in general, I only have suggestions to change wording in some parts.

I'm not very confident with backporting this change so late into 3.10's release cycle. I hope another core dev can review.

Comment threadLib/typing.py
cls = type(self)

if cls._is_protocol:
raise TypeError('Protocols cannot be instantiated')

@Fidget-SpinnerFidget-SpinnerSep 2, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note to self: this is the previous behavior.

Comment threadLib/typing.py Outdated
Comment threadLib/typing.py Outdated
Comment threadLib/test/test_dataclasses.py
uriyyoand others added 3 commits September 2, 2021 16:30
@uriyyo@Fidget-Spinner
Update Lib/test/test_dataclasses.py
6d1be74
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@uriyyo
Update Ptocol init method
91b011e
@uriyyo
Update comment
8dc6a2e
@julianfortunejulianfortune mentioned this pull request Sep 2, 2021
Closed
Fidget-Spinner
Fidget-Spinner approved these changes Sep 2, 2021
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I will wait a bit to see if Serhiy or Lukasz (or anyone else) says anything.

@Fidget-SpinnerFidget-Spinner changed the title bpo-45081: Fix dataclass __init__ method generation when inherit from Protocolbpo-45081: Fix __init__ method generation when inheriting from ProtocolSep 2, 2021
@ambv

ambv commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

The previous behavior was introduced in GH-27545 as an attempt to make non-protocol subclasses of Protocol ignore the __init__ inherited from protocol base classes.

This PR retains this ability and restores compatibility with dataclasses. LGTM.

@ambv
ambv merged commit 0635e20 into python:mainSep 2, 2021
@miss-islington

miss-islington commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

Thanks @uriyyo for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 2, 2021
@uriyyo@miss-islington
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
cc82f4f
…ol (pythonGH-28121)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)
Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Sep 2, 2021
@bedevere-bot

bedevere-bot commented Sep 2, 2021

Copy link
Copy Markdown

GH-28131 is a backport of this pull request to the 3.10 branch.

@bedevere-bot

bedevere-bot commented Sep 2, 2021

Copy link
Copy Markdown

GH-28132 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 2, 2021
@uriyyo@miss-islington
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
de94c9e
…ol (pythonGH-28121)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)
Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
ambv pushed a commit that referenced this pull request Sep 2, 2021
@miss-islington@uriyyo
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
98eb408
…ol (GH-28121) (GH-28132)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)
Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@leahwiczleahwicz mentioned this pull request Sep 2, 2021
Closed
4 tasks
miss-islington added a commit that referenced this pull request Sep 3, 2021
@miss-islington@uriyyo
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
79e9f5a
…ol (GH-28121)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)
Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@ronnie-llamadoronnie-llamado mentioned this pull request Sep 21, 2021
Closed
1 task
@JanEricNitschkeJanEricNitschke mentioned this pull request Sep 11, 2025
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericvsmithericvsmithericvsmith left review comments
@Fidget-SpinnerFidget-SpinnerFidget-Spinner approved these changes
@gvanrossumgvanrossumAwaiting requested review from gvanrossum

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@uriyyo@ambv@miss-islington@bedevere-bot@ericvsmith@Fidget-Spinner@the-knights-who-say-ni