Skip to content

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 11, 2024
Merged

gh-97959: Fix rendering of routines in pydoc #113941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9f790d2
gh-97959: Fix rendering of routines in pydoc
serhiy-storchaka Dec 11, 2023
8c60e21
Merge branch 'main' into pydoc-docroutine
serhiy-storchaka Jan 27, 2024
96a3e16
Merge branch 'main' into pydoc-docroutine
serhiy-storchaka Feb 4, 2024
73f26d2
Merge branch 'main' into pydoc-docroutine
serhiy-storchaka Feb 11, 2024
39410dd
Add *ignored in docmodule().
serhiy-storchaka Feb 11, 2024
c5d94e9
Merge remote-tracking branch 'refs/remotes/origin/pydoc-docroutine' i…
serhiy-storchaka Feb 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 115 additions & 34 deletions Lib/pydoc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,19 @@ def classname(object, modname):
name = object.__module__ + '.' + name
return name

def parentname(object, modname):
"""Get a name of the enclosing class (qualified it with a module name
if necessary) or module."""
if '.' in object.__qualname__:
name = object.__qualname__.rpartition('.')[0]
if object.__module__ != modname:
return object.__module__ + '.' + name
else:
return name
else:
if object.__module__ != modname:
return object.__module__
Comment on lines +237 to +239
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

Copy link
Member Author

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.


def isdata(object):
"""Check if an object is of a type that probably means it's data."""
return not (inspect.ismodule(object) or inspect.isclass(object) or
Expand Down Expand Up @@ -319,13 +332,15 @@ def visiblename(name, all=None, obj=None):
return not name.startswith('_')

def classify_class_attrs(object):
"""Wrap inspect.classify_class_attrs, with fixup for data descriptors."""
"""Wrap inspect.classify_class_attrs, with fixup for data descriptors and bound methods."""
results = []
for (name, kind, cls, value) in inspect.classify_class_attrs(object):
if inspect.isdatadescriptor(value):
kind = 'data descriptor'
if isinstance(value, property) and value.fset is None:
kind = 'readonly property'
elif kind == 'method' and _is_bound_method(value):
kind = 'static method'
results.append((name, kind, cls, value))
return results

Expand Down Expand Up @@ -681,6 +696,25 @@ def classlink(self, object, modname):
module.__name__, name, classname(object, modname))
return classname(object, modname)

def parentlink(self, object, modname):
"""Make a link for the enclosing class or module."""
link = None
name, module = object.__name__, sys.modules.get(object.__module__)
if hasattr(module, name) and getattr(module, name) is object:
if '.' in object.__qualname__:
name = object.__qualname__.rpartition('.')[0]
if object.__module__ != modname:
link = '%s.html#%s' % (module.__name__, name)
else:
link = '#%s' % name
else:
if object.__module__ != modname:
link = '%s.html' % module.__name__
Comment on lines +710 to +712
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else:
if object.__module__ != modname:
link = '%s.html' % module.__name__
elif object.__module__ != modname:
link = '%s.html' % module.__name__

if link:
return '<a href="%s">%s</a>' % (link, parentname(object, modname))
else:
return parentname(object, modname)

def modulelink(self, object):
"""Make a link for a module."""
return '<a href="%s.html">%s</a>' % (object.__name__, object.__name__)
Expand Down Expand Up @@ -925,7 +959,7 @@ def spill(msg, attrs, predicate):
push(self.docdata(value, name, mod))
else:
push(self.document(value, name, mod,
funcs, classes, mdict, object))
funcs, classes, mdict, object, homecls))
push('\n')
return attrs

Expand Down Expand Up @@ -1043,24 +1077,44 @@ def formatvalue(self, object):
return self.grey('=' + self.repr(object))

def docroutine(self, object, name=None, mod=None,
funcs={}, classes={}, methods={}, cl=None):
funcs={}, classes={}, methods={}, cl=None, homecls=None):
"""Produce HTML documentation for a function or method object."""
realname = object.__name__
name = name or realname
anchor = (cl and cl.__name__ or '') + '-' + name
if homecls is None:
homecls = cl
anchor = ('' if cl is None else cl.__name__) + '-' + name
note = ''
skipdocs = 0
skipdocs = False
imfunc = None
if _is_bound_method(object):
imclass = object.__self__.__class__
if cl:
if imclass is not cl:
note = ' from ' + self.classlink(imclass, mod)
imself = object.__self__
if imself is cl:
imfunc = getattr(object, '__func__', None)
elif inspect.isclass(imself):
note = ' class method of %s' % self.classlink(imself, mod)
else:
if object.__self__ is not None:
note = ' method of %s instance' % self.classlink(
object.__self__.__class__, mod)
else:
note = ' unbound %s method' % self.classlink(imclass,mod)
note = ' method of %s instance' % self.classlink(
imself.__class__, mod)
elif (inspect.ismethoddescriptor(object) or
inspect.ismethodwrapper(object)):
try:
objclass = object.__objclass__
except AttributeError:
pass
else:
if cl is None:
note = ' unbound %s method' % self.classlink(objclass, mod)
elif objclass is not homecls:
note = ' from ' + self.classlink(objclass, mod)
else:
imfunc = object
if inspect.isfunction(imfunc) and homecls is not None and (
imfunc.__module__ != homecls.__module__ or
imfunc.__qualname__ != homecls.__qualname__ + '.' + realname):
pname = self.parentlink(imfunc, mod)
if pname:
note = ' from %s' % pname

if (inspect.iscoroutinefunction(object) or
inspect.isasyncgenfunction(object)):
Expand All @@ -1071,10 +1125,13 @@ def docroutine(self, object, name=None, mod=None,
if name == realname:
title = '<a name="%s"><strong>%s</strong></a>' % (anchor, realname)
else:
if cl and inspect.getattr_static(cl, realname, []) is object:
if (cl is not None and
inspect.getattr_static(cl, realname, []) is object):
reallink = '<a href="#%s">%s</a>' % (
cl.__name__ + '-' + realname, realname)
skipdocs = 1
skipdocs = True
if note.startswith(' from '):
note = ''
else:
reallink = realname
title = '<a name="%s"><strong>%s</strong></a> = %s' % (
Expand Down Expand Up @@ -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):
Copy link
Member

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 🤔

Copy link
Member Author

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.

"""Produce html documentation for a data descriptor."""
results = []
push = results.append
Expand Down Expand Up @@ -1213,7 +1270,7 @@ def formattree(self, tree, modname, parent=None, prefix=''):
entry, modname, c, prefix + ' ')
return result

def docmodule(self, object, name=None, mod=None):
def docmodule(self, object, name=None, mod=None, *ignored):
"""Produce text documentation for a given module object."""
name = object.__name__ # ignore the passed-in name
synop, desc = splitdoc(getdoc(object))
Expand Down Expand Up @@ -1392,7 +1449,7 @@ def spill(msg, attrs, predicate):
push(self.docdata(value, name, mod))
else:
push(self.document(value,
name, mod, object))
name, mod, object, homecls))
return attrs

def spilldescriptors(msg, attrs, predicate):
Expand Down Expand Up @@ -1467,23 +1524,43 @@ def formatvalue(self, object):
"""Format an argument default value as text."""
return '=' + self.repr(object)

def docroutine(self, object, name=None, mod=None, cl=None):
def docroutine(self, object, name=None, mod=None, cl=None, homecls=None):
"""Produce text documentation for a function or method object."""
realname = object.__name__
name = name or realname
if homecls is None:
homecls = cl
note = ''
skipdocs = 0
skipdocs = False
imfunc = None
if _is_bound_method(object):
imclass = object.__self__.__class__
if cl:
if imclass is not cl:
note = ' from ' + classname(imclass, mod)
imself = object.__self__
if imself is cl:
imfunc = getattr(object, '__func__', None)
elif inspect.isclass(imself):
note = ' class method of %s' % classname(imself, mod)
else:
if object.__self__ is not None:
note = ' method of %s instance' % classname(
object.__self__.__class__, mod)
else:
note = ' unbound %s method' % classname(imclass,mod)
note = ' method of %s instance' % classname(
imself.__class__, mod)
elif (inspect.ismethoddescriptor(object) or
inspect.ismethodwrapper(object)):
try:
objclass = object.__objclass__
except AttributeError:
pass
else:
if cl is None:
note = ' unbound %s method' % classname(objclass, mod)
elif objclass is not homecls:
note = ' from ' + classname(objclass, mod)
else:
imfunc = object
if inspect.isfunction(imfunc) and homecls is not None and (
imfunc.__module__ != homecls.__module__ or
imfunc.__qualname__ != homecls.__qualname__ + '.' + realname):
pname = parentname(imfunc, mod)
if pname:
note = ' from %s' % pname

if (inspect.iscoroutinefunction(object) or
inspect.isasyncgenfunction(object)):
Expand All @@ -1494,8 +1571,11 @@ def docroutine(self, object, name=None, mod=None, cl=None):
if name == realname:
title = self.bold(realname)
else:
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):
Copy link
Member

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 []?

Copy link
Member Author

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".

Copy link
Contributor

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.

Copy link
Member Author

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.

skipdocs = True
if note.startswith(' from '):
note = ''
title = self.bold(name) + ' = ' + realname
argspec = None

Expand All @@ -1517,7 +1597,7 @@ def docroutine(self, object, name=None, mod=None, cl=None):
doc = getdoc(object) or ''
return decl + '\n' + (doc and self.indent(doc).rstrip() + '\n')

def docdata(self, object, name=None, mod=None, cl=None):
def docdata(self, object, name=None, mod=None, cl=None, *ignored):
"""Produce text documentation for a data descriptor."""
results = []
push = results.append
Expand All @@ -1533,7 +1613,8 @@ def docdata(self, object, name=None, mod=None, cl=None):

docproperty = docdata

def docother(self, object, name=None, mod=None, parent=None, maxlen=None, doc=None):
def docother(self, object, name=None, mod=None, parent=None, *ignored,
maxlen=None, doc=None):
"""Produce text documentation for a data object."""
repr = self.repr(object)
if maxlen:
Expand Down
48 changes: 47 additions & 1 deletion Lib/test/pydocfodder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

import types

def global_func(x, y):
"""Module global function"""

def global_func2(x, y):
"""Module global function 2"""

class A:
"A class."

Expand All @@ -26,7 +32,7 @@ def A_classmethod(cls, x):
"A class method defined in A."
A_classmethod = classmethod(A_classmethod)

def A_staticmethod():
def A_staticmethod(x, y):
"A static method defined in A."
A_staticmethod = staticmethod(A_staticmethod)

Expand Down Expand Up @@ -61,6 +67,28 @@ def BD_method(self):
def BCD_method(self):
"Method defined in B, C and D."

@classmethod
def B_classmethod(cls, x):
"A class method defined in B."

global_func = global_func # same name
global_func_alias = global_func
global_func2_alias = global_func2
B_classmethod_alias = B_classmethod
A_classmethod_ref = A.A_classmethod
A_staticmethod = A.A_staticmethod # same name
A_staticmethod_alias = A.A_staticmethod
A_method_ref = A().A_method
A_method_alias = A.A_method
B_method_alias = B_method
__repr__ = object.__repr__ # same name
object_repr = object.__repr__
get = {}.get # same name
dict_get = {}.get

B.B_classmethod_ref = B.B_classmethod


class C(A):
"A class, derived from A."

Expand Down Expand Up @@ -136,3 +164,21 @@ def __call__(self, inst):

submodule = types.ModuleType(__name__ + '.submodule',
"""A submodule, which should appear in its parent's summary""")

global_func_alias = global_func
A_classmethod = A.A_classmethod # same name
A_classmethod2 = A.A_classmethod
A_classmethod3 = B.A_classmethod
A_staticmethod = A.A_staticmethod # same name
A_staticmethod_alias = A.A_staticmethod
A_staticmethod_ref = A().A_staticmethod
A_staticmethod_ref2 = B().A_staticmethod
A_method = A().A_method # same name
A_method2 = A().A_method
A_method3 = B().A_method
B_method = B.B_method # same name
B_method2 = B.B_method
count = list.count # same name
list_count = list.count
get = {}.get # same name
dict_get = {}.get
10 changes: 5 additions & 5 deletions Lib/test/test_enum.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4851,22 +4851,22 @@ class Color(enum.Enum)
| The value of the Enum member.
|
| ----------------------------------------------------------------------
| Methods inherited from enum.EnumType:
| Static methods inherited from enum.EnumType:
|
| __contains__(value) from enum.EnumType
| __contains__(value)
| Return True if `value` is in `cls`.
|
| `value` is in `cls` if:
| 1) `value` is a member of `cls`, or
| 2) `value` is the value of one of the `cls`'s members.
|
| __getitem__(name) from enum.EnumType
| __getitem__(name)
| Return the member matching `name`.
|
| __iter__() from enum.EnumType
| __iter__()
| Return members in definition order.
|
| __len__() from enum.EnumType
| __len__()
| Return the number of members (no aliases)
|
| ----------------------------------------------------------------------
Expand Down
Loading