Stop generating a __text_signature__ that inspect cannot parse - #8681
Conversation
The interpreter supplies `callee` the way it supplies `vm`, but only `vm`
was filtered, so the argument surfaced as a Python parameter and left a
string inspect cannot parse:
>>> object.__getstate__.__text_signature__
'(*args, **kwargs, callee)'
>>> inspect.signature(object.__getstate__)
ValueError: builtin has invalid signature
Filter it in `func_sig`, and in `infer_native_call_flags`, which counted
it as a fixed positional argument. `object.__getstate__` is the only
#[pymethod] taking one today, inherited by 95 types.
Assisted-by: Claude Code:claude-opus-5
`from` is a Python keyword, so the generated signature could not be
parsed:
>>> bytes.maketrans.__text_signature__
'(from, to, /)'
>>> inspect.signature(bytes.maketrans)
ValueError: builtin has invalid signature
CPython's Argument Clinic spells it `frm` for the same reason. Both
parameters are positional-only, so the name is documentation only.
`bytes.maketrans` and `bytearray.maketrans` now report what CPython
reports, `(frm, to, /)`.
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: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe derive utility now excludes interpreter-supplied ChangesNative call and builtin cleanup
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This fixes invalid generated builtin signatures and aligns bytes and bytearray maketrans parameter names with Python syntax without changing their behavior. No concrete current-head merge risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
Thanks! This is a new feature to tell the function calling context. it seems we need to trim the convention more to avoid confusion. let's do it like this now.
…8681) * Leave Callee out of a generated __text_signature__ The interpreter supplies `callee` the way it supplies `vm`, but only `vm` was filtered, so the argument surfaced as a Python parameter and left a string inspect cannot parse: >>> object.__getstate__.__text_signature__ '(*args, **kwargs, callee)' >>> inspect.signature(object.__getstate__) ValueError: builtin has invalid signature Filter it in `func_sig`, and in `infer_native_call_flags`, which counted it as a fixed positional argument. `object.__getstate__` is the only #[pymethod] taking one today, inherited by 95 types. Assisted-by: Claude Code:claude-opus-5 * Rename maketrans's first parameter to frm `from` is a Python keyword, so the generated signature could not be parsed: >>> bytes.maketrans.__text_signature__ '(from, to, /)' >>> inspect.signature(bytes.maketrans) ValueError: builtin has invalid signature CPython's Argument Clinic spells it `frm` for the same reason. Both parameters are positional-only, so the name is documentation only. `bytes.maketrans` and `bytearray.maketrans` now report what CPython reports, `(frm, to, /)`. Assisted-by: Claude Code:claude-opus-5
2 participants
Summary
inspect.signature()raisesValueError: builtin has invalid signaturefor 99 method descriptors, because it parses__text_signature__as Python source and two generated strings are not valid Python. Two unrelated causes, one commit each.Calleewas reported as a parameter. The interpreter supplies it the way it suppliesvm, but onlyvmwas filtered:object.__getstate__is the only#[pymethod]taking one today, and 95 types inherit it.infer_native_call_flagscounted it as a fixed positional argument too, whichFuncArgshappens to short-circuit today, so the call flags were right by accident.maketransnamed its first parameterfrom, a Python keyword. Argument Clinic spells itfrmfor the same reason, sobytes.maketransandbytearray.maketransnow report(frm, to, /), which is what CPython reports. Both parameters are positional-only, so the name is documentation only.Notes
Calleeargument.__doc__counts are unchanged.Summary by CodeRabbit
Bug Fixes
Refactor