Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to)

@rhettinger

BPO	26103
Nosy	@rhettinger, @serhiy-storchaka, @1st1, @aaronchall, @Mariatta, @csabella, @chnlior
PRs	bpo-26103 resolve Data descriptor contradiction #1959 bpo-26103: Update the description of isdatadescriptor in inspect.rst #16645 [3.8] bpo-26103: Fix the description of isdatadescriptor in inspect.rst (GH-16645) #17180

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-03-20.20:16:37.395>
created_at = <Date 2016-01-13.20:56:52.079>
labels = ['3.7', '3.8', 'type-bug', 'library', 'docs']
title = 'Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to)'
updated_at = <Date 2019-11-15.21:49:05.856>
user = 'https://github.com/aaronchall'

bugs.python.org fields:

activity = <Date 2019-11-15.21:49:05.856>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2019-03-20.20:16:37.395>
closer = 'Aaron Hall'
components = ['Documentation', 'Library (Lib)']
creation = <Date 2016-01-13.20:56:52.079>
creator = 'Aaron Hall'
dependencies = []
files = []
hgrepos = []
issue_num = 26103
keywords = []
message_count = 12.0
messages = ['258168', '295212', '295243', '295287', '295288', '295307', '295378', '301206', '302721', '308078', '317211', '338426']
nosy_count = 8.0
nosy_names = ['rhettinger', 'docs@python', 'serhiy.storchaka', 'yselivanov', 'Aaron Hall', 'Mariatta', 'cheryl.sabella', 'chnlior']
pr_nums = ['1959', '16645', '17180']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26103'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']

Based on the data-model documentation (https://docs.python.org/2/reference/datamodel.html#invoking-descriptors) and the dotted lookup behavior, the follow definitions are correct:

"If the descriptor defines __set__() and/or __delete__(), it is a data descriptor; if it defines neither, it is a non-data descriptor."

def has_data_descriptor_attrs(obj):
    return set(['__set__', '__delete__']) & set(dir(obj))

def is_data_descriptor(obj):
    return bool(has_data_descriptor_attrs(obj))

However, the inspect module has the following, which is also reflected in the descriptor how-to (https://docs.python.org/2/howto/descriptor.html#descriptor-protocol):

"If an object defines both __get__() and __set__(), it is considered a data descriptor."

def isdatadescriptor(object):
    """Return true if the object is a data descriptor.

Data descriptors have both a __get__ and a __set__ attribute..."""
if isclass(object) or ismethod(object) or isfunction(object):
    # mutual exclusion
    return False
tp = type(object)
return hasattr(tp, "__set__") and hasattr(tp, "__get__")

I'm willing to sign a contributor release and fix myself.

Bumping this - I intend to work on this next, if no objections.

This isn't just a documentation issue since it fixes inspect.isdatadescriptor(). I confirm that the new implementation better matches the C code. LGTM, but needed tests for inspect.isdatadescriptor() and a Misc/NEWS entry.

Added news, working on tests

Please also add yourself to Misc/ACKS.

Please also add yourself to Misc/ACKS.

Done!

I tweaked the docs a little more this morning, but I believe I am done making any further changes unless so requested.

This issue doesn't say it's assigned to anyone. Is there anything else that needs to happen here?

Serhiy,

Not sure what else needs to be done to wrap this up. All checks are passing on the pull request.

Thoughts?

The only question is remained -- should *data descriptors* be *descriptors*? I.e. is the __get__ method required for data descriptors?

@serhiy

Joining @serhiy Storchaka last question.
Is the __get__ method existance is a must be a data descriptor?

According to the C implementation in descrobject.h

#define PyDescr_IsData(d) (Py_TYPE(d)->tp_descr_set != NULL)
#endif

the answer is No.
Does this C code reflect the true definition?

New changeset 4054b17 by Serhiy Storchaka (Aaron Hall, MBA) in branch 'master':
bpo-26103: Fix inspect.isdatadescriptor() and a data descriptor definition. (GH-1959)
4054b17

It looks like this issue can be closed now that it's merged?

I suspect that this issue was resolved in the wrong direction. It is meaningless and not useful to define something as a "data descriptor" that does not have a __get__ method, since the only implication of something being a "data descriptor" is that its __get__ method takes precedence over the instance dict. But obviously this "taking precedence" cannot occur when the descriptor has no __get__ method. So I think it is more useful to clarify that data descriptors must have both __get__ and either or both of __set__ and __delete__.

The reason PyDescr_IsData does not explicitly check for tp_descr_get is that, in every case where it's used, the accompanying step (either immediately before or after the PyDescr_IsData check) is to actually fetch tp_descr_get from the descriptor; if it is NULL, then attribute-getting always proceeds exactly as if the descriptor were not a data descriptor. (In fact, in 3/4 call sites the check is literally (descr_get != NULL && PyDescr_IsData(descr).) So also checking existence of tp_descr_get within PyDescr_IsData would be redundant in every place it is used. But the end-to-end behavior in all these cases is that descriptors without tp_descr_get behave exactly the same as non-data descriptors. I don't think we want PyDescr_IsData to do the redundant check (these are hot code paths; though tbh compilers might be smart enough to eliminate it anyway), maybe it should be renamed for better clarity?

I'm not sure whether this is important enough to file a new issue and fix. The main case where I can see it might cause confusion is that e.g. if inspect.getattr_static hewed blindly to the currently-documented definition of what makes a data descriptor, it would wrongly return the descriptor object instead of the instance dict value for descriptors without __get__. (Fortunately it doesn't follow the documented definition, but also checks for __get__.) This came up in #75367.

Navigation Menu

Repository navigation

Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to) #70291

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to) #70291

Description

Activity

aaronchall commented on Jan 13, 2016

aaronchall commented on Jun 5, 2017

serhiy-storchaka commented on Jun 6, 2017

aaronchall commented on Jun 6, 2017

Mariatta commented on Jun 6, 2017

aaronchall commented on Jun 6, 2017

aaronchall commented on Jun 8, 2017

aaronchall commented on Sep 4, 2017

serhiy-storchaka commented on Sep 21, 2017

chnlior commented on Dec 11, 2017

serhiy-storchaka commented on May 20, 2018

csabella commented on Mar 20, 2019

carljm commented on Apr 6, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions