Repository navigation
_json: skip items() for empty dict subclasses - #9018
Conversation
Split the Python 3.14-compatible empty native dictionary fast path from RustPython#8954. Extend the existing JSON snippet with root, nested, sorting, and nonempty subclass regression coverage. Assisted-by: OpenAI Codex:model identifier unavailable
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (2)
✨ 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 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05e06d0e3a
ℹ️ 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".
| out: &mut Wtf8Buf, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult<()> { | ||
| if dict.is_empty() { |
There was a problem hiding this comment.
Add the mandatory Assisted-by trailer
The commit message explicitly states that OpenAI Codex assisted, but it has no Assisted-by: AGENT_NAME:MODEL_VERSION trailer. Because repository policy requires that trailer for every AI-assisted contribution, add it before this commit lands rather than relying on the prose AI-assistance section.
AGENTS.md reference: AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| out: &mut Wtf8Buf, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult<()> { | ||
| if dict.is_empty() { |
There was a problem hiding this comment.
Run the required test suites instead of compile-checking
The Validation section explicitly says the workspace and C-API targets were built with --no-run and that broad tests were not executed; it also only mentions a focused snippet run. For a change to interpreter behavior and extra_tests/snippets, compile checks do not satisfy the repository minimum of running workspace tests, C-API tests, and pytest -v, so run those suites before landing.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| for sort_keys in (False, True): | ||
| assert json.dumps(EmptyDictWithItems(), sort_keys=sort_keys) == "{}" |
There was a problem hiding this comment.
Revert the unsupported test-file additions
Adding this loop and its assertions changes test logic in extra_tests/snippets/stdlib_json.py, but repository policy limits test-file modifications to removing an existing expected-failure/TODO when it passes or adding an expected-failure decorator. This regression block must therefore be removed from the change before landing.
AGENTS.md reference: AGENTS.md:L277-L279
Useful? React with 👍 / 👎.
Summary
Split the Python 3.14-compatible empty-dictionary encoder fix from #8954.
Check the native dictionary size before calling a subclass's
items()method. Empty dictionary subclasses should encode as{}without invoking that method, matching CPython 3.14. Nonempty subclasses keep their customitems()behavior.Extend the existing JSON snippet with empty-subclass cases at the root and inside a list, both sorting modes, and a nonempty-subclass control. No standard-library assertions change.
Validation
d0dfcb54through the snippet runner:items()was called on an empty dictionary subclass.TestCDump,TestCDefault, andTestCIndenttests pass unchanged.--no-run, excluding WASM, venvlauncher, and C API; the separate C-API test target also compiles with--no-run.Local runtime validation is limited to the focused snippet and canonical tests above. Broad Rust test targets were compile-checked, not executed.
Reference: CPython 3.14 dictionary encoder.
AI assistance
OpenAI Codex assisted with auditing the split, implementation, regression coverage, and validation. The runtime does not expose an exact model identifier.