← 返回首页
gh-102433: Add tests for how classes with properties interact with `isinstance()` checks on `typing.runtime_checkable` protocols by AlexWaygood · Pull Request #102449 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

gh-102433: Add tests for how classes with properties interact with isinstance() checks on typing.runtime_checkable protocols#102449

Merged
AlexWaygood merged 6 commits into
python:mainfrom
AlexWaygood:runtime-checkable-tests
Mar 11, 2023
Merged

gh-102433: Add tests for how classes with properties interact with isinstance() checks on typing.runtime_checkable protocols#102449
AlexWaygood merged 6 commits into
python:mainfrom
AlexWaygood:runtime-checkable-tests

Conversation

AlexWaygood commented Mar 5, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

It appears we currently have no tests for how classes with properties interact with isinstance checks for protocols decorated with @runtime_checkable.

I'm trying to only add tests here for uncontroversial behaviour that we won't want changed, whether or not any of the patches discussed in python/typing#1363 is implemented.

bedevere-bot added the tests Tests in the Lib/test dir label Mar 5, 2023
bedevere-bot mentioned this pull request Mar 5, 2023
Closed
AlexWaygood requested a review from sobolevn March 5, 2023 18:32
JelleZijlstra reviewed Mar 6, 2023
View reviewed changes
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved

AlexWaygood commented Mar 6, 2023

Copy link
Copy Markdown
Member Author

Marking as "DO-NOT-MERGE" for now, as this appears more controversial than I realised.

AlexWaygood marked this pull request as draft March 6, 2023 11:29
AlexWaygood marked this pull request as ready for review March 10, 2023 17:06

AlexWaygood commented Mar 10, 2023

Copy link
Copy Markdown
Member Author

I've removed the tests that assert undesirable behaviour, as per the consensus in python/typing#1363.

AlexWaygood requested a review from carljm March 10, 2023 17:07
AlexWaygood commented Mar 10, 2023
View reviewed changes
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
sobolevn reviewed Mar 10, 2023
View reviewed changes
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
Comment thread Lib/test/test_typing.py
class C:
@property
def attr(self):
return 42

sobolevn Mar 10, 2023

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 Hide comment

What about side-effects in @property like raise ValueError? Should we test this case?

AlexWaygood Mar 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 Hide comment

We should test this case, definitely! But the behaviour for this case may be about to change if any of the patches discussed in python/typing#1363 is implemented (and the consensus seems to be that we should implement one of those patches). If so, we should add the tests in the same PR as we change the behaviour.

Whether or not we decide to change the behaviour around properties that have side effects, I'd prefer to add those tests in a separate PR, so that this PR is solely focussed on adding tests for uncontroversial behaviour.

Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
AlexWaygood requested a review from sobolevn March 10, 2023 23:36
carljm approved these changes Mar 11, 2023
View reviewed changes

carljm 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 Hide comment

A couple nits, but these tests look good to me! Thank you!

Comment thread Lib/test/test_typing.py
class BadPG1(Protocol[T]):
attr: T

for obj in PG[T], PG[C], PG1[T], PG1[C], BadP, BadP1, BadPG, BadPG1:

carljm Mar 11, 2023

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 Hide comment

nit: these are all protocol classes still; the name 'protocol_class used in the above "good" loop seems much clearer than the name obj

AlexWaygood Mar 11, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 Hide comment

No strong opinion here, but the reason I avoided "protocol_class" for these ones is that the parameterised ones (PG[T], etc) aren't strictly speaking classes anymore — they're generic aliases to protocol classes.

Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
Update Lib/test/test_typing.py …
ba178f1
Co-authored-by: Carl Meyer <carl@oddbird.net>
AlexWaygood merged commit 5ffdaf7 into python:main Mar 11, 2023

miss-islington commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

AlexWaygood deleted the runtime-checkable-tests branch March 11, 2023 01:20
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 11, 2023
pythongh-102433: Add tests for how classes with properties interact w… …
6c77697
…ith `isinstance()` checks on `typing.runtime_checkable` protocols (pythonGH-102449) (cherry picked from commit 5ffdaf7) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Carl Meyer <carl@oddbird.net>

bedevere-bot commented Mar 11, 2023

Copy link
Copy Markdown

GH-102592 is a backport of this pull request to the 3.11 branch.

bedevere-bot removed the needs backport to 3.11 only security fixes label Mar 11, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 11, 2023
pythongh-102433: Add tests for how classes with properties interact w… …
385fae3
…ith `isinstance()` checks on `typing.runtime_checkable` protocols (pythonGH-102449) (cherry picked from commit 5ffdaf7) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Carl Meyer <carl@oddbird.net>

bedevere-bot commented Mar 11, 2023

Copy link
Copy Markdown

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

bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 11, 2023

AlexWaygood commented Mar 11, 2023

Copy link
Copy Markdown
Member Author

Thanks, all!

miss-islington added a commit that referenced this pull request Mar 11, 2023
gh-102433: Add tests for how classes with properties interact with `i… …
2810787
…sinstance()` checks on `typing.runtime_checkable` protocols (GH-102449) (cherry picked from commit 5ffdaf7) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Carl Meyer <carl@oddbird.net>
miss-islington added a commit that referenced this pull request Mar 11, 2023
gh-102433: Add tests for how classes with properties interact with `i… …
e467cb3
…sinstance()` checks on `typing.runtime_checkable` protocols (GH-102449) (cherry picked from commit 5ffdaf7) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Carl Meyer <carl@oddbird.net>
iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request Mar 12, 2023
pythongh-102433: Add tests for how classes with properties interact w… …
fde1a99
…ith `isinstance()` checks on `typing.runtime_checkable` protocols (python#102449) Co-authored-by: Carl Meyer <carl@oddbird.net>
AlexWaygood mentioned this pull request Apr 12, 2023
Merged
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

carljm carljm approved these changes
gvanrossum Awaiting requested review from gvanrossum
Fidget-Spinner Awaiting requested review from Fidget-Spinner
JelleZijlstra Awaiting requested review from JelleZijlstra JelleZijlstra is a code owner
sobolevn Awaiting requested review from sobolevn

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Footer

© 2026 GitHub, Inc.