gh-105834: Add tests for calling issubclass() between two protocols#105835
Uh oh!
There was an error while loading. Please reload this page.
Conversation
AlexWaygood requested review from Fidget-Spinner, JelleZijlstra and gvanrossum as code owners
carljm left a comment
carljm left a comment There was a problem hiding this comment.
Your reasoning on the issue looks right to me.
JelleZijlstra left a comment
JelleZijlstra left a comment There was a problem hiding this comment.
We need to improve our test coverage instead. This block covers a case like this (add this to test_basic_protocol):
diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 3eb0fcad69..84e9f2650f 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2474,6 +2474,11 @@ def f(): self.assertNotIsSubclass(types.FunctionType, P) self.assertNotIsInstance(f, P) + class HasAnno(Protocol): + meth: Callable[[Any], Any] + + self.assertIsSubclass(HasAnno, P) + def test_runtime_checkable_generic_non_protocol(self): # Make sure this doesn't raise AttributeError with self.assertRaisesRegex( On current main, this test passes, but with your patch applied, it no longer passes.
bedevere-bot commented Jun 16, 2023
When you're done making the requested changes, leave the comment: |
Thanks @JelleZijlstra! I knew I had to be missing something 😖 Here's a complete repro: >>> from typing import* >>> @runtime_checkable ... classCallableMembersProto(Protocol): ... defmeth(self): ... ... >>> classNonCallableMembersProto(Protocol): ... meth: Callable[[], None] ... >>> issubclass(NonCallableMembersProto, CallableMembersProto) TrueThe last line would be I can see arguments both ways in terms of whether it's desirable behaviour, and it seems really unlikely to come up in practice. (PEP-544 doesn't seem to discuss this point at all.) But given that this has been established behaviour since Python 3.8, I agree that we should probably keep it. (I also can't find any significant performance improvement from removing this block of code.) |
AlexWaygood changed the title typing.pyissubclass() between two protocolsAlexWaygood commented Jun 16, 2023
I have made the requested changes; please review again |
bedevere-bot commented Jun 16, 2023
Thanks for making the requested changes! @carljm, @JelleZijlstra: please review the changes made to this pull request. |
AlexWaygood added tests Uh oh!
There was an error while loading. Please reload this page.
miss-islington commented Jun 16, 2023
Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
miss-islington commented Jun 16, 2023
Sorry, @AlexWaygood, I could not cleanly backport this to |
bedevere-bot commented Jun 16, 2023
GH-105859 is a backport of this pull request to the 3.12 branch. |
…tocols (pythonGH-105835) Some parts of the implementation of `typing.Protocol` had poor test coverage (cherry picked from commit 70c075c) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
…two protocols (python#105835) Some parts of the implementation of `typing.Protocol` had poor test coverage
bedevere-bot commented Jun 16, 2023
GH-105860 is a backport of this pull request to the 3.11 branch. |
* main: pythongh-104799: PEP 695 backward compatibility for ast.unparse (python#105846) pythongh-105834: Add tests for calling `issubclass()` between two protocols (python#105835) CI: Remove docs build from Azure Pipelines (python#105823) pythongh-105844: Consistently use 'minor version' for X.Y versions (python#105851) Fix inaccuracies in "Assorted Topics" section of "Defining Extension Types" tutorial (python#104969) pythongh-105433: Add `pickle` tests for PEP695 (python#105443) bpo-44530: Document the change in MAKE_FUNCTION behavior (python#93189) pythonGH-103124: Multiline statement support for pdb (pythonGH-103125) pythonGH-105588: Add missing error checks to some obj2ast_* converters (pythonGH-105589)
…tocols (python#105835) Some parts of the implementation of `typing.Protocol` had poor test coverage
5 participants
typing.py#105834