-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
Create a primer section for the descriptor howto guide #22906
Conversation
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.
Is it possible to give more clarity on the attribute resolution method? The problem I had is that from this how-to it wasn't clear that assigning a descriptor as an attribute to a instance wouldn't make it's get and set methods resolved when that instance's attribute is called.
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.
A minor typo
Co-authored-by: Eduardo Orive Vinuesa <edorka@gmail.com>
Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
(cherry picked from commit 8d3d731) Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
GH-22918 is a backport of this pull request to the 3.9 branch. |
|
||
Descriptors get invoked by the dot operator during attribute lookup. If a | ||
descriptor is accessed indirectly with ``vars(some_class)[descriptor_name]``, | ||
the descriptor instance is returned without invoking it. |
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 wonder if it is worth mentioning here the more usual convention for returning the descriptor instance (checking if obj is None
)? The indirection vars(vars(Person)['name'])
is a little strange compared to vars(Person.name)
.
def newfunc(*args): | ||
return self.f(klass, *args) | ||
return self.f(cls, *args) | ||
return newfunc |
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.
Instead of using newfunc
, wouldn't it be better to return types.MethodType(self.f, cls)
similar to what is done in Function
?
klass = type(obj) | ||
def __get__(self, obj, cls=None): | ||
if cls is None: | ||
cls = type(obj) |
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.
The pure Python version doesn't capture the change #8405, which will call self.f.__get__(cls)
if self.f
has __get__
. I think this behavior is surprising (and a little weird) and should be captured in this document, which doesn't shy away from nitty gritty details. I also happen to think this behavior of calling f.__get__
is incomplete and should instead call self.f.__get__(cls, cls)
.
Work in progress. A couple of sections remain.