GeezerVison is showing github.com — Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to) · Issue #70291 · python/cpython
Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to) #70291Description
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'] Activityaaronchall commented on Jan 13, 2016 MannequinAuthor More actions 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. aaronchall commented on Jun 5, 2017 MannequinAuthor More actions Bumping this - I intend to work on this next, if no objections. serhiy-storchaka commented on Jun 6, 2017 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. aaronchall commented on Jun 6, 2017 MannequinAuthor More actions Added news, working on tests Mariatta commented on Jun 6, 2017 aaronchall commented on Jun 6, 2017 MannequinAuthor More actions aaronchall commented on Jun 8, 2017 MannequinAuthor More actions 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? aaronchall commented on Sep 4, 2017 MannequinAuthor More actions Serhiy,
Not sure what else needs to be done to wrap this up. All checks are passing on the pull request.
Thoughts? serhiy-storchaka commented on Sep 21, 2017 The only question is remained -- should *data descriptors* be *descriptors*? I.e. is the __get__ method required for data descriptors? chnlior commented on Dec 11, 2017 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? serhiy-storchaka commented on May 20, 2018 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 csabella commented on Mar 20, 2019 It looks like this issue can be closed now that it's merged? carljm commented on Apr 6, 2023 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.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|