-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-97959: Fix rendering of routines in pydoc #113941
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
gh-97959: Fix rendering of routines in pydoc #113941
Conversation
* Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes.
Run |
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.
Thank you! The scale of the internal pydoc change is rather big and complex. It is really hard to tell if all corner cases are covered.
But, tests look very diverse to cover them 👍
A few nitpicks.
else: | ||
if object.__module__ != modname: | ||
return object.__module__ |
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.
else: | |
if object.__module__ != modname: | |
return object.__module__ | |
elif object.__module__ != modname: | |
return object.__module__ |
This function can also return None
, not sure if this is desired 🤔
In that case it would render as from None
in https://github.com/python/cpython/pull/113941/files#diff-d2599cb4ccbf37634c5d3089a2750fc6c92c6c98d2a65861ff13d08ed3e9cff5R713-R714
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.
In the current form both branches of if '.' in object.__qualname__:
are more symmetric.
And this function always returns non-None if link is not-None.
else: | ||
if object.__module__ != modname: | ||
link = '%s.html' % module.__name__ |
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.
else: | |
if object.__module__ != modname: | |
link = '%s.html' % module.__name__ | |
elif object.__module__ != modname: | |
link = '%s.html' % module.__name__ |
@@ -1102,7 +1159,7 @@ def docroutine(self, object, name=None, mod=None, | |||
doc = doc and '<dd><span class="code">%s</span></dd>' % doc | |||
return '<dl><dt>%s</dt>%s</dl>\n' % (decl, doc) | |||
|
|||
def docdata(self, object, name=None, mod=None, cl=None): | |||
def docdata(self, object, name=None, mod=None, cl=None, *ignored): |
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.
Not sure why *ignored
is needed 🤔
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.
Because homecls
is now passed, but it is only used in docroutine()
. Perhaps mod
and cl
could be included in it, as they are not used too. Some other methods already defined with *ignored
parameter.
self.assertIn(' | __repr__(self, /) from builtins.object', lines) | ||
self.assertIn(' | object_repr = __repr__(self, /)', lines) | ||
|
||
lines = self.getsection(result, f' | Static methods {where}:', ' | ' + '-'*70) |
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.
'-'*70
can probably be a test helper.
if cl and inspect.getattr_static(cl, realname, []) is object: | ||
skipdocs = 1 | ||
if (cl is not None and | ||
inspect.getattr_static(cl, realname, []) is 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.
The default value here is rather strange. Why is 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.
Because it is the simplest method to create an unique not shared value. [] is object
is always false, so this condition means "the class have such attribute and it is the same as 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.
ISTM like that piece of information should be recorded as a comment in the code.
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.
This is a common idiom. Well, usually it is used in more complex code:
sentinel = [...]
result = getattr(obj, name, sentinel)
if result is sentinel: ...
But in this particular case no need to introduce variables for the result and the sentinel, so it all can be simplified.
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry, @serhiy-storchaka, I could not cleanly backport this to
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to
|
…13941) * Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes. (cherry picked from commit 2939ad0) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-115296 is a backport of this pull request to the 3.12 branch. |
…15296) * Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes. (cherry picked from commit 2939ad0)
…honGH-113941) (pythonGH-115296) * Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes. (cherry picked from commit 2939ad0) (cherry picked from commit cfb79ca) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-115296) (GH-115302) * Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes. (cherry picked from commit 2939ad0) (cherry picked from commit cfb79ca)
* Class methods no longer have "method of builtins.type instance" note. * Corresponding notes are now added for class and unbound methods. * Method and function aliases now have references to the module or the class where the origin was defined if it differs from the current. * Bound methods are now listed in the static methods section. * Methods of builtin classes are now supported as well as methods of Python classes.
pydoc
rendersfrom builtins.type
note, even if it is incorrect #97959