GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Match Element.remove argument type diagnostics by youknowdot · Pull Request #9068 · RustPython/RustPython

Conversation

youknowdot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Current revision

Targets main after #8950, with one logical commit: 39feb175a4f6d541055cad830ebfa8ab5e9aa8fd (parent d88c9350fb5de38b8c3be74ba8b53c8dd68a9b8f). The unique patch is byte-identical to the prior reviewed head and now includes main's merged #9082 shared CI fix; original author attribution and assistance trailers are retained. No files under Lib/ are changed.

Normal hooks, whitespace checks, and independent tree/history review passed. The temporary [skip ci] marker is removed and normal CI is enabled for this final head. No new local build or runtime test is claimed. Validation recorded below belongs to earlier revisions unless explicitly stated otherwise.

Rebased once after #9082 merged to repair the shared Python 3.15 IO reference failure. Normal commit hooks and exact range-diff/patch-equivalence checks pass. Exact-head hosted CI is running; no additional local runtime validation is claimed.

Summary

Extract the independent Element.remove argument diagnostic from #8954, adapted to the exact CPython 3.14 contract:

Report None specially and use the native type name for other invalid arguments.
Match the 50-byte type-name limit without splitting a UTF-8 character.
Preserve accepted Element subclasses, the identity/equality/reentrant removal loop, and the existing missing-child ValueError.

This does not include the adjacent 3.15 missing-child message change or any XML parser changes.

Sources: CPython 3.14.8 clinic wrapper, _PyArg_BadArgument, and UTF-8 precision handling. The relevant 3.14.7 contract is identical.

Validation

Independent source and final-diff review; normal commit hooks.
Focused six-case Python snippet passes CPython 3.14.7 and 3.14.8 and fails on the prior native baseline.
Native build and the six diagnostic cases passed; reference/native snippet harness: 2 pytest cases.
Four unchanged canonical accelerator removal/reentrancy tests passed. The direct unittest CLI hit the existing import-depth limit while loading asyncio through unittest.mock; preloading those dependencies allowed the same tests to run without changing the recursion limit or any assertion.
Stdlib and separate C-API Clippy checks with warnings denied.

No XML parser was constructed or executed by these focused tests. Full workspace suites and non-Linux runtime tests were not run locally. hosted CI is temporarily skipped.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: b84f2ff2-a3fc-469f-a77f-244d5cc0d367

📥 Commits

Reviewing files that changed from the base of the PR and between d88c935 and 39feb17.

📒 Files selected for processing (2)

crates/stdlib/src/elementtree.rs
extra_tests/snippets/stdlib_xml.py

 _________________________________________________________________________
< Congratulations, you implemented a distributed single point of failure. >
 -------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone reviewed

Oct 10, 2026

View reviewed changes

Comment thread

extra_tests/snippets/stdlib_elementtree.py Outdated

youknowone Oct 10, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdlib_xml.py

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowdot Oct 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the six cases unchanged into stdlib_xml.py in de32170 and removed the separate snippet. The moved block is AST-identical and passes focused CPython 3.14.7/3.14.8 and native checks. Full XML snippet execution is not newly claimed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowdot force-pushed the fix-element-remove-type-error branch 2 times, most recently from 71e4bfa to ba210d6 Compare

October 11, 2026 03:21

youknowone changed the base branch from main to stack/python-3.15-8950

October 11, 2026 03:25

youknowdot force-pushed the fix-element-remove-type-error branch from ba210d6 to 63be507 Compare

October 11, 2026 04:07

youknowone changed the base branch from stack/python-3.15-8950 to main

October 11, 2026 04:09

youknowdot force-pushed the fix-element-remove-type-error branch from 63be507 to 294f2f3 Compare

October 11, 2026 04:14

github-actions Bot commented Oct 11, 2026

Copy link

Copy Markdown

Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_mmap.py (TODO: 19)

dependencies:

dependent tests: (2 tests)

mmap: test_os
- multiprocessing.shared_memory: test_genericalias

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Match Element.remove argument type diagnostics

39feb17

Match native type names, None spelling, and UTF-8 byte limits while preserving removal behavior. Keep all six regression cases in the existing XML snippet.

Assisted-by: OpenAI Codex:model identifier unavailable

youknowdot force-pushed the fix-element-remove-type-error branch from 294f2f3 to 39feb17 Compare

October 11, 2026 13:29

codspeed Bot commented Oct 11, 2026

Copy link

Copy Markdown

Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing youknowdot:fix-element-remove-type-error (39feb17) with main (d88c935)}

Footnotes

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone approved these changes

Oct 11, 2026

View reviewed changes

youknowone marked this pull request as ready for review

October 11, 2026 14:47

youknowone merged commit 1fa883b into RustPython:main

Oct 11, 2026

21 checks passed

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-11T14:51:59.017512Z	`39feb17`	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot reviewed

Oct 11, 2026

View reviewed changes

chatgpt-codex-connector Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 39feb175a4

ℹ️ 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".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

extra_tests/snippets/stdlib_xml.py

		]

		root = Element("root")
		for value, type_name in (

chatgpt-codex-connector Bot Oct 11, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the newly added test logic

This hunk adds test data, control flow, and an assertion to extra_tests/snippets/stdlib_xml.py, but the repository's critical test-file policy permits only adding or removing expectedFailure decorators; this regression coverage therefore needs to be removed rather than extending the snippet.

AGENTS.md reference: AGENTS.md:L275-L279

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Reviewers

chatgpt-codex-connector[bot]

chatgpt-codex-connector[bot] left review comments

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

youknowdot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current revision

Summary

Validation

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

youknowone Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

youknowdot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 11, 2026

📦 Library Dependencies

Uh oh!

codspeed Bot commented Oct 11, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

youknowdot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.