Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
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 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
* 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 commentedJan 16, 2024
Run |
serhiy-storchaka commentedFeb 4, 2024
sobolevn left a comment
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: | ||
| ifobject.__module__!=modname: | ||
| returnobject.__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: | |
| ifobject.__module__!=modname: | |
| returnobject.__module__ | |
| elifobject.__module__!=modname: | |
| returnobject.__module__ |
This function can also returnNone, not sure if this is desired 🤔
In that case it would render asfrom None inhttps://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 ofif '.' in object.__qualname__: are more symmetric.
And this function always returns non-None if link is not-None.
| else: | ||
| ifobject.__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: | |
| ifobject.__module__!=modname: | |
| link='%s.html'%module.__name__ | |
| elifobject.__module__!=modname: | |
| link='%s.html'%module.__name__ |
| return'<dl><dt>%s</dt>%s</dl>\n'% (decl,doc) | ||
| defdocdata(self,object,name=None,mod=None,cl=None): | ||
| defdocdata(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.
Becausehomecls is now passed, but it is only used indocroutine(). Perhapsmod andcl 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.
| ifclandinspect.getattr_static(cl,realname, [])isobject: | ||
| skipdocs=1 | ||
| if (clisnotNoneand | ||
| inspect.getattr_static(cl,realname, [])isobject): |
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)ifresultissentinel: ...
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 commit2939ad0)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-115296 is a backport of this pull request to the3.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 commit2939ad0)
…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 commit2939ad0)(cherry picked from commitcfb79ca)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 commit2939ad0)(cherry picked from commitcfb79ca)
* 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.
Uh oh!
There was an error while loading.Please reload this page.
pydocrendersfrom builtins.typenote, even if it is incorrect #97959