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

bpo-44807: Allow Protocol classes to define __init__ #31628

Merged
merged 7 commits into from
Apr 11, 2022
Merged

bpo-44807: Allow Protocol classes to define __init__ #31628

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e21200b
Allow Protocol classes to define __init__
adriangb Mar 1, 2022
fb419d1
📜🤖 Added by blurb_it.
blurb-it[bot] Mar 2, 2022
1fe020a
split up and improve test
adriangb Mar 2, 2022
9c22a44
Update Lib/test/test_typing.py
adriangb Mar 2, 2022
282e95f
Update Misc/NEWS.d/next/Library/2022-03-02-04-25-58.bpo-44807.gHNC9J.rst
adriangb Mar 2, 2022
28cf196
Update Lib/test/test_typing.py
adriangb Mar 2, 2022
e2840af
Update Lib/test/test_typing.py
JelleZijlstra Mar 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,26 @@ class CG(PG[T]): pass
with self.assertRaises(TypeError):
CG[int](42)

def test_protocol_defining_init(self):
class P(Protocol):
x: int
def __init__(self, x: int) -> None:
self.x = x

# the protocol itself cannot be instantiated
with self.assertRaises(TypeError):
P()
Copy link
Member

Choose a reason for hiding this comment

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

But presumably P(1) will work?

Copy link
Contributor Author

@adriangb adriangb Mar 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Great catch. This is a bad assertion with an even worse comment: this TypeError is occurring because it's missing an argument, not because of the no-instantiation check. I'm inclined to just remove this assertRaises. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Agree that the current test doesn't make much sense. It would be useful though to add a test that asserts that P.__init__ doesn't get clobbered. For example, you could call P.__init__(some_object, 1) directly.

(Also, unrelatedly, this change will need a NEWS entry.)

Copy link
Contributor Author

@adriangb adriangb Mar 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the feedback.

I split up the test into 2:

  1. Verify that __init__ is no longer clobbered (as per your suggestion)
  2. Check that concrete subclasses can inherit and use init from a protocol class


# but a concrete subclass can
class C(P): pass

c = C(1)
self.assertIsInstance(c, C)

# and this concrete subclass can inherit the __init__
# implementation from Protocol
self.assertEqual(c.x, 1)

def test_cannot_instantiate_abstract(self):
@runtime_checkable
class P(Protocol):
Expand Down
3 changes: 2 additions & 1 deletion Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,8 @@ def _proto_hook(other):
issubclass(base, Generic) and base._is_protocol):
raise TypeError('Protocols can only inherit from other'
' protocols, got %r' % base)
cls.__init__ = _no_init_or_replace_init
if cls.__init__ is Protocol.__init__:
cls.__init__ = _no_init_or_replace_init


class _AnnotatedAlias(_GenericAlias, _root=True):
Expand Down