Skip to content

Backport python/cpython#31628 #119

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.

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

Closed
wants to merge 1 commit into from

Conversation

Gobot1234
Copy link
Contributor

@Gobot1234 Gobot1234 commented Feb 20, 2023

Needs decision on how to fix failing test

Fixes #104

@JelleZijlstra JelleZijlstra self-requested a review February 21, 2023 00:48
@JelleZijlstra
Copy link
Member

cc @adriangb if you're interested.

@JelleZijlstra
Copy link
Member

Looking at some of the failing tests:

  • 3.7 is totally broken because of the inheritance from typing.Generic. This needs to be fixed. We probably need to preserve the existing 3.7 implementation.
  • 3.10 fails on class X(Generic[T], Protocol):. If Protocol[T] works we can live without that, but would be nice to support both ways.
  • 3.10 allows class P(Protocol[T, T]): pass. Not the end of the world but a little odd.

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 12, 2023

@Gobot1234, following recent changes I've made to typing_extensions.Protocol over the last few days, I think you can get away with a much simpler patch now. This patch passes all tests locally for me, on Python 3.11 and Python 3.7 (the diff is against main, not against this PR branch):

diff --git a/src/test_typing_extensions.py b/src/test_typing_extensions.py
index 3a483b5..6fcfafb 100644
--- a/src/test_typing_extensions.py
+++ b/src/test_typing_extensions.py
@@ -1454,6 +1454,32 @@ class ProtocolTests(BaseTestCase):
         class CG(PG[T]): pass
         self.assertIsInstance(CG[int](), CG)

+    def test_protocol_defining_init_does_not_get_overridden(self):
+        # check that P.__init__ doesn't get clobbered
+        # see https://bugs.python.org/issue44807
+
+        class P(Protocol):
+            x: int
+            def __init__(self, x: int) -> None:
+                self.x = x
+        class C: pass
+
+        c = C()
+        P.__init__(c, 1)
+        self.assertEqual(c.x, 1)
+
+    def test_concrete_class_inheriting_init_from_protocol(self):
+        class P(Protocol):
+            x: int
+            def __init__(self, x: int) -> None:
+                self.x = x
+
+        class C(P): pass
+
+        c = C(1)
+        self.assertIsInstance(c, C)
+        self.assertEqual(c.x, 1)
+
     def test_cannot_instantiate_abstract(self):
         @runtime_checkable
         class P(Protocol):
diff --git a/src/typing_extensions.py b/src/typing_extensions.py
index c28680c..94218ac 100644
--- a/src/typing_extensions.py
+++ b/src/typing_extensions.py
@@ -662,7 +662,8 @@ else:
                         isinstance(base, _ProtocolMeta) and base._is_protocol):
                     raise TypeError('Protocols can only inherit from other'
                                     f' protocols, got {repr(base)}')
-            cls.__init__ = _no_init
+            if cls.__init__ is Protocol.__init__:
+                cls.__init__ = _no_init

@Gobot1234
Copy link
Contributor Author

I may have forgotten about this PR 😅. That does look a lot nicer though.

@AlexWaygood
Copy link
Member

I may have forgotten about this PR 😅. That does look a lot nicer though.

Happy to take over if you like, or leave it to you -- whatever you prefer :)

@Gobot1234
Copy link
Contributor Author

If you already have it working that'd be great, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

typing.Protocol in 3.11 needs to be backported
3 participants