Report every generated __text_signature__, and keep it out of __doc__ - #8614
Conversation
A function's signature and its documentation share one string, separated
by `\n--\n\n`, as in CPython. __text_signature__ read the front of it,
but __doc__ returned the whole thing:
>>> len.__doc__
'len(obj, /)\n--\n\nReturn the number of items in a container.' # was
'Return the number of items in a container.' # now
44 of the 46 builtin functions leaked the prefix.
get_doc_from_internal_doc already stripped it but was reachable only from
PyType.__doc__. Split it the way CPython does, into doc_without_signature
(_PyType_DocWithoutSignature) and a wrapper mapping an empty result to
None (_PyType_GetDocFromInternalDoc), and call it from PyNativeFunction
and PyMethodDescriptor as well.
Assisted-by: Claude Code:claude-opus-5
A signature and its documentation share one string, separated by
`\n--\n\n`, and readers find the signature by searching for that marker.
A #[pyfunction] with no doc comment stored the bare signature, which no
reader could parse:
>>> time.clock_getres.__doc__
'clock_getres(clk_id, /)'
>>> time.clock_getres.__text_signature__
None
Emit the marker in that case too, as Argument Clinic does for an
undocumented function. 136 of the 915 functions reachable from the
importable modules were affected.
__doc__ now reports None for them, matching CPython, because nothing
follows the marker.
Assisted-by: Claude Code:claude-opus-5
A #[pymethod] built its doc only when the method carried a doc comment,
so one without a comment dropped the generated signature too:
>>> list.append.__text_signature__
None
Build it from either part, as #[pyfunction] does. All 1101 method
descriptors reachable from the builtin types now report a signature, up
from 1, and 713 of the 1030 that CPython also describes match it exactly.
A method that takes its receiver as an ordinary first argument instead of
`&self` reported that argument by name, so binding the method could not
drop it:
>>> inspect.signature([].__dir__)
(obj, /) # was
() # now
Mark it $self, or $type for a classmethod, as CPython does. 310 methods
were affected.
test_unbound_builtin_method_noargs and test_bound_builtin_method_noargs
pass now.
Assisted-by: Claude Code:claude-opus-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughChangesBuilt-in signature and documentation handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change corrects built-in documentation and signature metadata without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MethodItem
participant text_signature
participant func_sig
participant SignatureTests
MethodItem->>text_signature: pass implicit_self marker
text_signature->>func_sig: generate signature arguments
func_sig-->>MethodItem: return formatted signature
MethodItem->>SignatureTests: expose generated signature
SignatureTests->>SignatureTests: assert bound and unbound signatures
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/pydoc.py dependencies:
dependent tests: (5 tests)
[ ] lib: cpython/Lib/concurrent dependencies:
dependent tests: (17 tests)
[x] test: cpython/Lib/test/test_descr.py (TODO: 31) dependencies: dependent tests: (no tests depend on descr) [ ] test: cpython/Lib/test/test_import (TODO: 3) dependencies: dependent tests: (no tests depend on import) [ ] test: cpython/Lib/test/test_class.py (TODO: 13) dependencies: dependent tests: (no tests depend on class) [x] test: cpython/Lib/test/test_cmd_line_script.py (TODO: 14) dependencies: dependent tests: (no tests depend on cmd_line_script) [ ] test: cpython/Lib/test/test_pyrepl (TODO: 21) dependencies: dependent tests: (no tests depend on pyrepl) Legend:
|
|
rlcompleter closes the parenthesis for a callable whose signature takes
no parameters, and leaves it open when the signature cannot be read:
def _callable_postfix(self, val, word):
if callable(val):
word += "("
try:
if not inspect.signature(val).parameters:
word += ")"
except ValueError:
pass
os.getpid reports `()` since the generated signature carries the `--`
terminator, so completing `os.getpid` yields `os.getpid()` and the test
passes.
Assisted-by: Claude Code:claude-opus-5
A method that needs an owned handle cannot take `&self`, so it takes the receiver as its first argument and names it `zelf`, `self` being a Rust keyword. 24 of the 163 such methods named it something else: `instance` (11), `obj` (4), `exc` (3), `_self` (3), `_instance` (2). Rename them. The generated signature is unaffected either way, since the macro marks the receiver by position rather than by name. Assisted-by: Claude Code:claude-opus-5
The receiver is now marked by position, so the name it carries no longer matters, and no path reaches this check with an argument named zelf: - a #[pymethod] or #[pyclassmethod] without a `&self` receiver has its first argument replaced by the marker before the name is read - a `&self` receiver is handled as syn::FnArg::Receiver - a #[pymethod(raw)] is passed to static_raw_func, whose PyNativeFn bound fixes its signature to (&VirtualMachine, FuncArgs) - no #[pystaticmethod] or #[pyfunction] names an argument zelf - #[pymember], #[pygetset] and #[pyslot] never reach func_sig Every generated signature is byte for byte identical without it. Assisted-by: Claude Code:claude-opus-5
A #[pyclassmethod] that takes FuncArgs receives the class inside the
bundle along with every other argument, but the marker was spent on the
bundle and the arguments went unreported:
>>> object.__subclasshook__.__text_signature__
'($type, /)' # was, claims it takes nothing
'($type, *args, **kwargs)' # now
Report both, the shape CPython uses for __new__. Two methods take their
arguments this way, __subclasshook__ on object and on type.
Assisted-by: Claude Code:claude-opus-5
__text_signature__, and keep it out of __doc__
youknowone
left a comment
There was a problem hiding this comment.
looks great, thank you so much! 👍
…c__` (#8614) * Strip the signature prefix from builtin __doc__ A function's signature and its documentation share one string, separated by `\n--\n\n`, as in CPython. __text_signature__ read the front of it, but __doc__ returned the whole thing: >>> len.__doc__ 'len(obj, /)\n--\n\nReturn the number of items in a container.' # was 'Return the number of items in a container.' # now 44 of the 46 builtin functions leaked the prefix. get_doc_from_internal_doc already stripped it but was reachable only from PyType.__doc__. Split it the way CPython does, into doc_without_signature (_PyType_DocWithoutSignature) and a wrapper mapping an empty result to None (_PyType_GetDocFromInternalDoc), and call it from PyNativeFunction and PyMethodDescriptor as well. Assisted-by: Claude Code:claude-opus-5 * Terminate a generated __text_signature__ that carries no documentation A signature and its documentation share one string, separated by `\n--\n\n`, and readers find the signature by searching for that marker. A #[pyfunction] with no doc comment stored the bare signature, which no reader could parse: >>> time.clock_getres.__doc__ 'clock_getres(clk_id, /)' >>> time.clock_getres.__text_signature__ None Emit the marker in that case too, as Argument Clinic does for an undocumented function. 136 of the 915 functions reachable from the importable modules were affected. __doc__ now reports None for them, matching CPython, because nothing follows the marker. Assisted-by: Claude Code:claude-opus-5 * Report a __text_signature__ for every method A #[pymethod] built its doc only when the method carried a doc comment, so one without a comment dropped the generated signature too: >>> list.append.__text_signature__ None Build it from either part, as #[pyfunction] does. All 1101 method descriptors reachable from the builtin types now report a signature, up from 1, and 713 of the 1030 that CPython also describes match it exactly. A method that takes its receiver as an ordinary first argument instead of `&self` reported that argument by name, so binding the method could not drop it: >>> inspect.signature([].__dir__) (obj, /) # was () # now Mark it $self, or $type for a classmethod, as CPython does. 310 methods were affected. test_unbound_builtin_method_noargs and test_bound_builtin_method_noargs pass now. Assisted-by: Claude Code:claude-opus-5 * Drop the expectedFailure on test_simple_completion rlcompleter closes the parenthesis for a callable whose signature takes no parameters, and leaves it open when the signature cannot be read: def _callable_postfix(self, val, word): if callable(val): word += "(" try: if not inspect.signature(val).parameters: word += ")" except ValueError: pass os.getpid reports `()` since the generated signature carries the `--` terminator, so completing `os.getpid` yields `os.getpid()` and the test passes. Assisted-by: Claude Code:claude-opus-5 * Name every #[pymethod] receiver zelf A method that needs an owned handle cannot take `&self`, so it takes the receiver as its first argument and names it `zelf`, `self` being a Rust keyword. 24 of the 163 such methods named it something else: `instance` (11), `obj` (4), `exc` (3), `_self` (3), `_instance` (2). Rename them. The generated signature is unaffected either way, since the macro marks the receiver by position rather than by name. Assisted-by: Claude Code:claude-opus-5 * Drop the unreachable zelf name check in func_sig The receiver is now marked by position, so the name it carries no longer matters, and no path reaches this check with an argument named zelf: - a #[pymethod] or #[pyclassmethod] without a `&self` receiver has its first argument replaced by the marker before the name is read - a `&self` receiver is handled as syn::FnArg::Receiver - a #[pymethod(raw)] is passed to static_raw_func, whose PyNativeFn bound fixes its signature to (&VirtualMachine, FuncArgs) - no #[pystaticmethod] or #[pyfunction] names an argument zelf - #[pymember], #[pygetset] and #[pyslot] never reach func_sig Every generated signature is byte for byte identical without it. Assisted-by: Claude Code:claude-opus-5 * Keep the argument bundle when marking the receiver A #[pyclassmethod] that takes FuncArgs receives the class inside the bundle along with every other argument, but the marker was spent on the bundle and the arguments went unreported: >>> object.__subclasshook__.__text_signature__ '($type, /)' # was, claims it takes nothing '($type, *args, **kwargs)' # now Report both, the shape CPython uses for __new__. Two methods take their arguments this way, __subclasshook__ on object and on type. Assisted-by: Claude Code:claude-opus-5
Summary
A generated signature and a function's documentation share one string, separated by
\n--\n\n, as in CPython. Defects in writing and reading that string left__doc__carrying the signature and nearly every method carrying none.__doc__still has the signature line__text_signature__What changed
__doc__returned the whole string while__text_signature__read only the front of it, solen.__doc__was'len(obj, /)\n--\n\nReturn the number of items in a container.'.get_doc_from_internal_docalready stripped the prefix but was reachable only fromPyType.__doc__; split it the way CPython does, intodoc_without_signature(_PyType_DocWithoutSignature) and a wrapper mapping an empty result toNone(_PyType_GetDocFromInternalDoc), and call it fromPyNativeFunctionandPyMethodDescriptoras well.#[pyfunction]stored the bare signature, leaving no--marker for a reader to find:time.clock_getres.__doc__was'clock_getres(clk_id, /)'and its__text_signature__wasNone. Emit the marker in that case too, as Argument Clinic does for an undocumented function.#[pymethod]built its doc only when the method carried a doc comment, dropping the generated signature along with it. The signature comes from the Rust parameter list, not from the comment, so it was already being built and then thrown away. Build the doc from either part, as#[pyfunction]does.&selfwould otherwise report it as a plain parameter, and binding could not drop it. Mark it$selffor 310 method descriptors and$typefor 211 classmethod entries, as CPython does, soinspect.signature([].__dir__)is()rather than(zelf, /). A method takingFuncArgsreceives the receiver inside that bundle, so it reports both:object.__subclasshook__is($type, *args, **kwargs).Left for #8383
317 of the 1030 methods CPython also describes still differ, in two groups needing different work.
#[derive(FromArgs)]struct orFuncArgsthe generator cannot see into, soint.from_bytesreports($type, args, /)where CPython reports($type, /, bytes, byteorder='big', *, signed=False).ArgAttributealready parses each field's name, kind and default for argument binding; teachingFromArgsto report them would close this.list.appendreportingxwhere CPython reportsobject. Mechanical, and independent of the above.Notes
Lib/test/test_pydoc/test_pydoc.pyandLib/test/test_pyrepl/test_pyrepl.pydrop threeexpectedFailuremarkers that pass now.rlcompletercloses the parenthesis for a callable whose signature takes no parameters, so completingos.getpidyieldsos.getpid().PyWrapperhas no__text_signature__getter, and its docs are hand-written rather than generated, so there is nothing to strip.#[pymethod]receivers are now uniformly namedzelf, andfunc_sig's name check for it is gone. Both are cosmetic: the receiver is marked by position, so every generated signature is byte for byte identical without them.Summary by CodeRabbit
New Features
None.Bug Fixes
Tests