Repository navigation
Check percent-c character lengths before index conversion - #9052
youknowone merged 1 commit into
Conversation
Assisted-by: OpenAI Codex:model identifier unavailable
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The change makes Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: validating %ccharacter lengths before attempting index conversion.Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8134a701b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert b"%o" % PercentInt(3) == b"7" | ||
|
|
||
|
|
||
| def test_percent_character_subclasses(): |
There was a problem hiding this comment.
This adds a new test function and assertions to an existing test file, but the repository policy limits test-file changes to adding expected-failure decorators or removing them together with their TODO comments; keep the %c fix in Rust without adding this test logic.
AGENTS.md reference: AGENTS.md:L273-L279
Useful? React with 👍 / 👎.
2 participants
Summary
Extract the small
%ccharacter-subclass dispatch fix from #8954 (43065e79ddc46eafeca58cb39398ac007b5d25f7) onto Python 3.14 main (3af5f8a15314446d2b28a90e429c938dc7b8beeb).Empty or multi-character str subclasses, and empty or multi-byte bytes/bytearray subclasses, must raise the existing length-specific
TypeErrorbefore considering__index__. Currently those objects fall through to numeric conversion. One-character payloads and non-character numeric fallback remain unchanged.The existing 3.14 error constructors are reused without adopting the mega's 3.15 messages or broad streaming-parser rewrite. The bytearray read guard is released before constructing its error. References: CPython 3.14.7 formatchar and byte_converter.
Related merged #8876, #7769 and #8600 are already in the base. Their numeric conversion and existing diagnostics are preserved. Open #8950 retains this fallthrough behavior, so this is not a duplicate of its argument-context changes.
Validation
__index__; this head passes the full snippet and its pytest entry. Native full-snippet runs had a 1.5-GiB address-space limit after reviewing its existing oversized-allocation error paths.This is only character-family length precedence; pre-existing numeric callback-error normalization and qualified-name diagnostic differences are outside its scope. No canonical assertions, README text, implementation-name guards or Rust behavioral tests were added or changed. Local checks are bounded Linux checks, not a complete workspace/cross-platform claim. Full hosted CI is pending; this remains a draft for review.
AI-assisted implementation and validation with OpenAI Codex; model identifier unavailable. No human review is implied.
Summary by CodeRabbit
%cformatting now reports clear type errors for empty or multi-character strings, bytes, and bytearrays. Valid single-character values and integer-like inputs continue to work.%cformatting with subclasses of strings, bytes, and bytearrays.