-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-102936: typing: document performance pitfalls of protocols decorated with @runtime_checkable
#102937
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
gh-102936: typing: document performance pitfalls of protocols decorated with @runtime_checkable
#102937
Conversation
…untime_checkable`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! Had one suggestion for a different example of attribute
@@ -1584,16 +1584,31 @@ These are not used in annotations. They are building blocks for creating generic | |||
|
|||
assert isinstance(open('/some/file'), Closable) | |||
|
|||
@runtime_checkable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think many users aren't too familiar with OSError
's constructor / we can't use kwargs and it's unclear what the second argument is. Maybe we could use int
and HasNumerator
instead?
(I know it's not writeable, but OSError
's errno also is maybe not writeable? Writing to errno doesn't affect repr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was really struggling to think of a good example for this! I'll ponder some more and see if I can come up with something better...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps unsurprisingly, not too many good cases of writeable attributes in stdlib. Maybe Thread.name? It's explicitly documented as something you can write to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works! I'll make the change tomorrow (it's late here)
Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
…ecorated with `@runtime_checkable` (pythonGH-102937) (cherry picked from commit 58d2b30) Co-authored-by: Alex Waygood <[email protected]>
GH-102963 is a backport of this pull request to the 3.11 branch. |
…ecorated with `@runtime_checkable` (pythonGH-102937) (cherry picked from commit 58d2b30) Co-authored-by: Alex Waygood <[email protected]>
GH-102964 is a backport of this pull request to the 3.10 branch. |
Thanks all for the reviews! |
…ed with `@runtime_checkable` (GH-102937) (cherry picked from commit 58d2b30) Co-authored-by: Alex Waygood <[email protected]>
…ed with `@runtime_checkable` (GH-102937) (cherry picked from commit 58d2b30) Co-authored-by: Alex Waygood <[email protected]>
…ecorated with `@runtime_checkable` (python#102937)
…ecorated with `@runtime_checkable` (python#102937)
@runtime_checkable
#102936