Repository navigation
Reuse checked signatures for deque, timezone and type - #9773
youknowdot wants to merge 6 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. |
|
|
||
| // timezone | ||
|
|
||
| /// timezone(offset, name=<unrepresentable>) |
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
The mechanism looks right: has_internal_doc_signature follows find_signature + skip_signature in Objects/typeobject.c (the doc must start with name(, the scan looks for )\n--\n\n, and it gives up at a blank line). These four objects get their text signatures from hand-written docstring headers in CPython as well. A few things should change before merging, though.
Observed before → after
Before: a build of 45a012cda (main at the time). After: the values asserted in this PR's snippet, which match CPython 3.15.
| object | __text_signature__ before |
after (= CPython) | inspect.signature |
|---|---|---|---|
deque |
None |
'([iterable[, maxlen]])' |
ValueError both |
deque.index |
'($self, needle, start=0, stop=<unrepresentable>, /)' |
'($self, value, [start, [stop]])' |
ValueError both |
timezone |
None |
'(offset, name=<unrepresentable>)' |
ValueError both |
type.__dict__['__prepare__'] |
'($type, name, bases, /, **kwds)' |
'($cls, name, bases, /, **kwds)' |
(type, …) → (cls, …) |
In practice this changes raw metadata only. The only inspect-visible difference is the receiver name of the unbound __prepare__.
1. Drop "legacy" from the naming; one comment is factually wrong
The feature is "an explicit internal-doc signature in the docstring is used as-is", and has_internal_doc_signature already says that. "Legacy" in the title, branch, code comment, test name and snippet comment does not describe it:
- The comment in
signature.rs, "Explicit Clinic headers cover legacy signatures that are not Python parameter lists", is wrong for__prepare__:($cls, name, bases, /, **kwds)is an ordinary parameter list. Only the deque headers use optional groups. Something like: "A doc that starts with a completename(...)\n--\n\nheader supplies its own text signature." explicit_internal_doc_preserves_legacy_signatures→ e.g.explicit_internal_doc_signature_is_kept.- Snippet comment "Legacy Clinic headers …" and the
legacy signature should remain unrepresentablemessage → describe them as explicit internal-doc signatures. - PR title → e.g. "Honor explicit internal-doc signatures for deque, timezone and type.prepare".
#9757 went through the same rename (legacy_signature → signature_from_flags) for the same reason.
2. The header is used implicitly and never checked against the Rust binding
Any /// doc that happens to be header-shaped now silently replaces the generated signature, and nothing checks the hand-written header against the real arguments. The drift is already there: deque.index binds needle in Rust, while the header says value. The displayed text is correct today, but a later change to the Rust signature would go unnoticed.
#9757 takes the opposite approach: an explicit #[pyfunction(signature_from_flags)] opt-in, plus const assertions that reject a mismatched arity at compile time. CPython does not validate hand-written headers either, so this is not a parity bug. It is still inconsistent within RustPython. Please consider at least a const check that the header's positional arity agrees with the binding metadata, or an explicit opt-in so the override cannot happen by accident.
3. Docs are now split between two sources
Because a /// doc overrides the crates/doc DB entry, the bodies of these four docstrings are now hard-coded in Rust, and regenerating crates/doc/src/data.inc.rs will no longer update them. crates/doc/generate.py stores only __doc__, which has the header stripped. A broader fix would be to have the generator also capture __text_signature__. That can be a follow-up, but it should be acknowledged here.
4. Conflict with #9757
Both PRs append to crates/vm/src/function/signature.rs and to the end of extra_tests/snippets/builtin_signature.py, so whichever lands second needs a rebase.
— commented by Claude
a6fadd3 to
847ee0b
Compare
Keep optional-group and CSV display signatures separate from argument binding. Validate explicit overrides against binding metadata in a const context and preserve legacy call behavior. Assisted-by: OpenAI Codex: model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
847ee0b to
632530b
Compare
youknowdot
changed the title
Assisted-by: OpenAI Codex:model version not exposed
Summary
Depends on #9756's explicit checked
text_signaturesupport. This is now an actual dependent follow-up, not a second implicit docstring-header mechanism. Once #9756 merges, this PR's diff can be reduced to the remaining extensions and four applications.rustpython-docdatabase; no production documentation text is duplicated or edited.text_signaturefor class implementations, validated against initializer/constructor argument metadata. An explicit class display may retain an unrepresentable default without passing the ordinary representability filter./. Verify arity, requiredness, receiver markers, and keyword names; omitted kind notation leaves the actual binder authoritative.$clsas a display alias of a class receiver. For known unrestricted*args, **kwargsbinders only, preserve the pre-existing declaredPARAMScontract rather than imposing a different binder. Unknown binding metadata remains an error.deque,deque.index,timezone, andtype.__prepare__. Argument conversion, dispatch, and runtime validation remain unchanged. All additional checking/composition happens at compile time.type.__prepare__still ignores arbitrary supplied arguments and returns a fresh namespace.CPython 3.15 basis
_collectionsmodule.c: deque constructor and index explicitly use Argument Clinic@text_signatureoverrides, independently of their actual parameter kinds._datetimemodule.c: timezone's optional name has a NULL default, displayed as<unrepresentable>.type_prepareignores all arguments; its method table publishes$cls, name, bases, /, **kwdsseparately.Validation
c07e0bc954.builtin_signature.py: passes on the new RustPython binary and CPython 3.15.2; all four raw signatures and complete documentation bodies match exactly.-m test test_deque test_datetime test_inspect: success, 1,622 tests with 152 skips.extra_testspytest: 495 passed, 20 failed, 15 subtests passed. Re-running the same failures against the prior baseline reproduces all 20 failures exactly (sandbox socket/IPC permissions, CPython distribution/test-package/version expectations, and_sqlite3omitted from the default build). This is not a full-suite green claim.AI assistance
OpenAI Codex assisted with review, implementation, and validation.