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.

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

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 11, 2024

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

* 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.
@serhiy-storchaka
Copy link
Member Author

Run ./python -m pydoc test.pydocfolder to see how different types of methods are rendered now.

@serhiy-storchaka
Copy link
Member Author

@sobolevn, please take a look at this PR. It is much larger than your #98120 and fixes deeper issues. I am not sure that I completely satisfied by its result, there will be more changes in pydoc.

Copy link
Member

@sobolevn sobolevn left a 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.

Comment on lines +237 to +239
else:
if object.__module__ != modname:
return object.__module__
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.

Comment on lines +710 to +712
else:
if object.__module__ != modname:
link = '%s.html' % module.__name__
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__

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

self.assertIn(' | __repr__(self, /) from builtins.object', lines)
self.assertIn(' | object_repr = __repr__(self, /)', lines)

lines = self.getsection(result, f' | Static methods {where}:', ' | ' + '-'*70)
Copy link
Member

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):
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.

@serhiy-storchaka serhiy-storchaka merged commit 2939ad0 into python:main Feb 11, 2024
@serhiy-storchaka serhiy-storchaka deleted the pydoc-docroutine branch February 11, 2024 13:19
@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Feb 11, 2024
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 2939ad02be62110ffa2ac6c4d9211c85e1d1720f 3.11

@miss-islington-app
Copy link

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 2939ad02be62110ffa2ac6c4d9211c85e1d1720f 3.12

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Feb 11, 2024
…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>
@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

GH-115296 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Feb 11, 2024
serhiy-storchaka added a commit that referenced this pull request Feb 11, 2024
…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)
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Feb 11, 2024
…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>
@serhiy-storchaka serhiy-storchaka removed their assignment Feb 11, 2024
@serhiy-storchaka serhiy-storchaka removed the needs backport to 3.11 only security fixes label Feb 11, 2024
serhiy-storchaka added a commit that referenced this pull request Feb 11, 2024
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)
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull request Feb 14, 2024
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants