Repository navigation
Complete PyTime conversion fixtures and repair existing helpers - #9317
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Stack the eight consumed conversion APIs on the existing-helper prerequisite in RustPython#9317. Keep the arithmetic local to the real fixture consumers; do not expose _testcapi or change sleep/lifecycle behavior. Assisted-by: Codex:unknown
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Extract the already-exposed conversion helper prerequisite from the broader RustPython#9143 surface without adding new test C-API exports or changing time.sleep. Assisted-by: Codex:unknown
Stack the eight consumed conversion APIs on the existing-helper prerequisite in RustPython#9317. Keep the arithmetic local to the real fixture consumers; do not expose _testcapi or change sleep/lifecycle behavior. Assisted-by: Codex:unknown
2940485 to
4aee9e9
Compare
Merging this PR will improve performance by 13.6%
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
Summary
Complete the coherent 11-helper PyTime conversion family in
_testinternalcapi:_PyTime_AsTimeval_clamp,_PyTime_AsTimespec, and_PyTime_AsTimespec_clamp. Honor all four rounding modes, negative values, ties-to-even, carries, native bounds, and CPython wrapper argument validation/precedence. DeriveSIZEOF_TIME_Tfrom the host type._PyTime_FromSeconds,_PyTime_FromSecondsObject,_PyTime_AsTimeval,_PyTime_AsMilliseconds,_PyTime_AsMicroseconds,_PyTime_ObjectToTime_t,_PyTime_ObjectToTimeval, and_PyTime_ObjectToTimespec.This intentionally extracts the PyTime fixture/API prerequisite from the broader work in #9143 and #8954. The original three-helper repair and the additional eight fixtures are kept together in this PR rather than publishing a duplicate dependent PR. When #9143 is rebased, preserve or reuse these contracts instead of replacing them with less strict wrappers.
No
_testcapinamespace, unused shared exports,time.sleeprefactor, lifecycle changes, or Lib edits are introduced. These are Python-facing fixture implementations, not a claim of C-ABI coverage.Validation
Pinned reference: CPython 3.15.0 final,
d220409a13b77de7a6c30abe16ea90db01528edc. Oracle interpreter, libpython and_testinternalcapiextension hashes are bound to the retained build/source provenance.Validated final commit:
29404850a370aad85467a782b65d7b44349a5f62.capifeature passes.-D warnings).--no-default-featurescheck passes, with 18 warnings in unchanged code; this is not a warning-free gate.The original baseline binary was main
0b0a3607plus the AST-Single warning patch from #9303. Its entire_testinternalcapi.rsmatched the PR's main94f92e4abase, and the exec-mode integer matrix does not exercise that warning route. This is not a claim of exact-current-main baseline or fresh-main integration testing.Local runtime validation is Linux 64-bit. Integer 32-bit time_t probes and source-reviewed platform branches do not establish Windows/32-bit execution; float neighbors around ±2**31 are not covered. Custom conversion callbacks and subclasses are not exercised by these finite matrices.
Canonical PyTime classes require
_testcapieven for the internal helpers. Their guards are unchanged, and skipped classes are not counted as passing coverage. No callback, sleep, subprocess, or lifecycle tests are claimed.AI assistance was used in preparing this change and its validation.
Summary by CodeRabbit
Rebase validation (2026-10-24)
Rebased onto main
1cd1b5d09f4853b2f13a5f3d5d6f856e901b671eas4aee9e991297300a664593293d05bdb37316acdb. Both original commits are unchanged ingit range-diff, and the full-index patch is byte-identical. No Lib, test, sleep, or lifecycle changes were added.cargo check, strict threaded VM Clippy (-D warnings), and the default-feature CLI build passed on the exact rebased tree.