-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
gh-136492: Add FrameLocalsProxy
to types
#136546
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
base: main
Are you sure you want to change the base?
Changes from all commits
78b842c
e2ddcbe
22e056d
b8fd733
abbe1d3
753c9bc
b087d9b
1971a14
fdeb2d0
c8600b9
70cc4db
fee5256
a110148
929f81e
fb710eb
5923263
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,7 @@ def _m(self): pass | |
EllipsisType = type(Ellipsis) | ||
NoneType = type(None) | ||
NotImplementedType = type(NotImplemented) | ||
FrameLocalsProxyType = (lambda: type(sys._getframe().f_locals))() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Introspecting this class currently would make you think it is
What do you think about changing its Personally I'd ideally want to do this for all the types.* types, but there are compatibility concerns for some. Here though, given that the type is relatively new and private, maybe we can get away with changing its name and module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense to me, any objections @gaogaotiantian? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean if we are keeping the behavior of almost all the types in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel the fact that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with |
||
|
||
# CapsuleType cannot be accessed from pure Python, | ||
# so there is no fallback definition. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add :data:`~types.FrameLocalsProxyType` to the :mod:`types` module. |
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.
This should be fixable by converting these methods to Argument Clinic or manually adding a docstring with a signature.
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.
To be clear I'm not asking you to necessarily fix this, though I think it'd be nice to reduce the size of the denylist in this test.
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 agree that we should fix these, but would you like it to be done in this PR? Same goes for the
tp_doc
onFrameLocalsProxy
.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.
Up to you! I feel it makes sense though that if we expose this publicly, we should also give it a docstring.
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'll add the
tp_doc
in this PR, but I think the AC changes would be better for a follow-up.