Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
bpo-20285: Improve help docs for object#4759
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Wouldn't the beginners be confused by the term "superclass"?
| @@ -873,7 +873,7 @@ def spilldata(msg, attrs, predicate): | |||
| thisclass = attrs[0][2] | |||
| attrs, inherited = _split_list(attrs, lambda t: t[2] is thisclass) | |||
| if thisclass is builtins.object: | |||
| ifobject is not builtins.object andthisclass is builtins.object: | |||
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.
What is the purpose of this change?
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 havehelp(object) print the methods defined onobject, the same as it does for every other class.
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.
One confusing thing about this module is that the argument that the docs are being created for is named isobject. So, for this PR, object both can refer to the name of the argument and the class called object.
thisclass is the third value (index 2) in the sequence returned frominspect.classify_class_attrs(object), which is defined as "The class which defined this attribute" (from the docstring). When the value of the class isobject, the current if statementif thisclass is builtins.object: results in that attribute being skipped from the output.
The change is to skip the logic when the argument value is not builtins.object so that the attributes will be displayed whenhelp(object) is used. Without this change, all of the attributes are skipped because "The class which defined this attribute" is alwaysobject.
Objects/typeobject.c Outdated
| @@ -4663,6 +4663,8 @@ static PyMethodDef object_methods[] = { | |||
| {0} | |||
| }; | |||
| PyDoc_STRVAR(object_doc, | |||
| "object()\n--\n\nThe superclass for all Python classes."); | |||
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.
for -> of
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 me, 'of' has more of an implication of immediate superclass. But see my response to Serhiy.
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.
help(someclass) prints someclass.doc and then prints the methods defined 'here' (on someclass). The goal of the issue is to improve 'help(object)' in two ways.
Replace the current docstring. I think my initial proposal needs at least one word added and further changes are up for discussion.
Actually print the methods defined on object. The patch lists the same methods as returned by dir(object). The pydoc change looks correct to me. I presume that the format of the typeobject change is correct since Serhiy did not say otherwise.
bedevere-bot commentedDec 9, 2017
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
@terryjreedy I don't quite understand what changes you'd like me to make. I changed the docstring to match what you asked about on the bug tracker for#1, but I'm not sure what you mean by 'Actually print the methods defined on object.'. Thanks! |
Your initial patch fixed the help output. The second has the wording I suggested on the issue. I will expand the news entry a bit. |
Absent an even better suggestion, I would like to go with this docstring, resulting from pydev discussion (see the tracker). "The base class of the class hierarchy. When called, it accepts no arguments and returns a new featureless instance that has no instance attributes and cannot be given any." |
Objects/typeobject.c Outdated
| @@ -4707,6 +4707,9 @@ static PyMethodDef object_methods[] = { | |||
| {0} | |||
| }; | |||
| PyDoc_STRVAR(object_doc, | |||
| "object()\n--\n\nThe starting base class of all types and classes other\n" | |||
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 am not familiar with docstrings in C code, but it appears that 'classname()\n--\n\n' gets removed before making the rest into the class objectdoc attribute.
Objects/typeobject.c Outdated
| @@ -4707,6 +4707,9 @@ static PyMethodDef object_methods[] = { | |||
| {0} | |||
| }; | |||
| PyDoc_STRVAR(object_doc, | |||
| "object()\n--\n\nThe starting base class of all types and classes other\n" | |||
| "than itself."); | |||
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 first line of a docstring, if not too long, should not have an embedded \n. So I think this should have been written
"object()\n--\n\n"
"The starting base class of all types and classes other than itself\n"
My suggested replacement would then be
"object()\n--\n\n"
"The base class of the class hierarchy.\n\n"
"When called, it accepts no arguments and returns a new featureless\n"
"instance that has no instance attributes and cannot be given any.\n"
A limit of 72 columns with a 4 column indent allows 68 chars. The longest above is 66.
@terryjreedy I've applied the changes. Thanks! |
help(object) now looks correct on my machine, after the update merge. Rather than run test suite locally, I pushed to have patch to core object tested with current code on all CI systems. I believe there have been other patches to object.c since last CI run. |
Thanks@terryjreedy! It looks like all the CI tests have passed, so I"m going to merge this. |
Uh oh!
There was an error while loading.Please reload this page.
Improve the docstring for
objectand showobjectmethods when usinghelp(object).https://bugs.python.org/issue20285