Skip to content
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

Inaccurate emulation of _PyType_Lookup() in the Descriptor HOWTO #95822

Closed
iyume opened this issue Aug 9, 2022 · 2 comments
Closed

Inaccurate emulation of _PyType_Lookup() in the Descriptor HOWTO #95822

iyume opened this issue Aug 9, 2022 · 2 comments
Assignees
Labels
docs Documentation in the Doc dir

Comments

@iyume
Copy link
Contributor

iyume commented Aug 9, 2022

Documentation

Doc link: https://docs.python.org/3.10/howto/descriptor.html#invocation-from-an-instance

Source: https://github.com/python/cpython/blame/eb81c1aea16914347919745e843c982ed831a9fb/Doc/howto/descriptor.rst#L585-L601

The code example is bad at https://github.com/python/cpython/blame/eb81c1aea16914347919745e843c982ed831a9fb/Doc/howto/descriptor.rst#L589

getattr(objtype, name) will go through descriptor.__get__.

Code Snippet:

class A:
    def __get__(self, obj, objtype):
        return obj, objtype

class B:
    a = A()

print(B().a)
# (<__main__.B object at 0x7ff56d0915e0>, <class '__main__.B'>)
print(B.a)  # same as getattr
# (None, <class '__main__.B'>)

Solution

Using __dict__ (or vars) instead of getattr:

print(vars(B)['a'])
# <__main__.A object at 0x7ff56cfef2b0>
@rhettinger
Copy link
Contributor

rhettinger commented Aug 10, 2022

Thanks for the bug report. Here's a proposed update that replaces getattr(objtype, name) with find_name_in_mro(objtype, name, null):

def find_name_in_mro(cls, name, default):
    "Emulate _PyType_Lookup() in Objects/typeobject.c"
    for base in cls.__mro__:
        if name in vars(base):
            return vars(base)[name]
    return default

def object_getattribute(obj, name):
    "Emulate PyObject_GenericGetAttr() in Objects/object.c"
    null = object()
    objtype = type(obj)
    cls_var = find_name_in_mro(objtype, name, null)    
    descr_get = getattr(type(cls_var), '__get__', null)
    if descr_get is not null:
        if (hasattr(type(cls_var), '__set__')
            or hasattr(type(cls_var), '__delete__')):
            return descr_get(cls_var, obj, objtype)     # data descriptor
    if hasattr(obj, '__dict__') and name in vars(obj):
        return vars(obj)[name]                          # instance variable
    if descr_get is not null:
        return descr_get(cls_var, obj, objtype)         # non-data descriptor
    if cls_var is not null:
        return cls_var                                  # class variable
    raise AttributeError(name)

Also, I want to add tests to cover your example and to cover why the fix proposed in the PR was incorrect.

@rhettinger rhettinger changed the title Doc: descriptior HOWTO bad example Inaccurate emulation of _PyType_Lookup() in the Descriptor HOWTO Aug 10, 2022
rhettinger added a commit to rhettinger/cpython that referenced this issue Aug 13, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 19, 2022
…lent. (pythonGH-95967)

(cherry picked from commit 6740680)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 19, 2022
…lent. (pythonGH-95967)

(cherry picked from commit 6740680)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
@rhettinger
Copy link
Contributor

Thanks for the report. This is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants