FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-45081: Fix __init__ method generation when inheriting from Protocol by uriyyo · Pull Request #28121 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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
ConversationCommits (5)ChecksFiles changed
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 commented Sep 2, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

uriyyo requested a review from ericvsmith as a code owner September 2, 2021 09:45

uriyyo commented Sep 2, 2021

Copy link
Copy Markdown
Member Author

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

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

Fidget-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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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 thread Lib/dataclasses.py Outdated
uriyyo requested a review from gvanrossum as a code owner September 2, 2021 12:03

uriyyo commented Sep 2, 2021

Copy link
Copy Markdown
Member Author

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

Fidget-Spinner reviewed Sep 2, 2021
View reviewed changes

Fidget-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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 thread Lib/typing.py
cls = type(self)

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

Fidget-Spinner Sep 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Note to self: this is the previous behavior.

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

Fidget-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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Fidget-Spinner changed the title bpo-45081: Fix dataclass __init__ method generation when inherit from Protocol bpo-45081: Fix __init__ method generation when inheriting from Protocol Sep 2, 2021

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 merged commit 0635e20 into python:main Sep 2, 2021

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
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-bot removed the needs backport to 3.10 only security fixes label Sep 2, 2021

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 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
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
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>
leahwicz mentioned this pull request Sep 2, 2021
Closed
2 of 4 tasks
miss-islington added a commit that referenced this pull request Sep 3, 2021
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-llamado mentioned this pull request Sep 21, 2021
Closed
1 task done
JanEricNitschke mentioned this pull request Sep 11, 2025
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ericvsmith ericvsmith left review comments
Fidget-Spinner Fidget-Spinner approved these changes
gvanrossum Awaiting 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

Back | FazBrowse Home | New Git URL