Skip to content

gh-123213: Fixed xml.etree.ElementTree.Element.extend and assignment to no longer hide exceptions #123214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Aug 23, 2024
Merged

gh-123213: Fixed xml.etree.ElementTree.Element.extend and assignment to no longer hide exceptions #123214

merged 9 commits into from
Aug 23, 2024

Conversation

bharel
Copy link
Contributor

@bharel bharel commented Aug 22, 2024 •
edited
Loading

Backport to 3.12 please.

Not formatting the exception message with the type (i.e "expected iterable not '...'") per PySequence_Fast standard usage across all modules.

Apparently this bug exists for 15 years ☹️

Typeshed already expects an Iterable for .extend() and __setitem__, so it does not require an update ❤️

@bedevere-app bedevere-app bot mentioned this pull request Aug 22, 2024
Closed
picnixz
picnixz reviewed Aug 22, 2024
View reviewed changes
@picnixz picnixz self-requested a review August 22, 2024 11:14
picnixz
picnixz reviewed Aug 22, 2024
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As I said previously, your argument is sound. However, one could argue that list.extend raises a TypeError with the incorrect type in the error message. I don't mind either suggestions (technically, the message could include the type information but you'll probably need to play with some PyErr_ExceptionMatches or remove the usage of PySequence_Fast and do something different). I personally would like feedback from someone else so that we could arrive at the best solution both for the user and for the maintainability.

bharel and others added 2 commits August 22, 2024 23:34
@bharel @picnixz
Update Modules/_elementtree.c
93c5b4a 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bharel
tests
555d5ef
@bharel
Copy link
Contributor Author

bharel commented Aug 22, 2024

@eliben our xml expert 🙇
Quick one for the morning 😄

@picnixz
Copy link
Member

picnixz commented Aug 23, 2024

I'm not sure that eli is still active/available but we can ask @serhiy-storchaka for this one.

serhiy-storchaka
serhiy-storchaka approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Aug 23, 2024
@serhiy-storchaka serhiy-storchaka merged commit 90b6d0e into python:main Aug 23, 2024
41 checks passed
@miss-islington-app
Copy link

Thanks @bharel for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 23, 2024
@bharel @miss-islington
pythongh-123213: Fixed xml.etree.ElementTree.Element.extend and assig…
84735b7 
…nment to no longer hide exceptions (pythonGH-123214)

(cherry picked from commit 90b6d0e)

Co-authored-by: Bar Harel <bharel@barharel.com>
@bedevere-app
Copy link

bedevere-app bot commented Aug 23, 2024

GH-123257 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 23, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 23, 2024
@bharel @miss-islington
pythongh-123213: Fixed xml.etree.ElementTree.Element.extend and assig…
6ede9d4 
…nment to no longer hide exceptions (pythonGH-123214)

(cherry picked from commit 90b6d0e)

Co-authored-by: Bar Harel <bharel@barharel.com>
@bedevere-app
Copy link

bedevere-app bot commented Aug 23, 2024

GH-123258 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Aug 23, 2024
@bharel bharel deleted the gh-123213 branch August 23, 2024 09:21
Yhg1s pushed a commit that referenced this pull request Sep 2, 2024
@miss-islington @bharel
[3.13] gh-123213: Fixed xml.etree.ElementTree.Element.extend and assi…
d9e4c4b 
…gnment to no longer hide exceptions (GH-123214) (#123257)

gh-123213: Fixed xml.etree.ElementTree.Element.extend and assignment to no longer hide exceptions (GH-123214)
(cherry picked from commit 90b6d0e)

Co-authored-by: Bar Harel <bharel@barharel.com>
hugovk pushed a commit that referenced this pull request Sep 6, 2024
@miss-islington @bharel
[3.12] gh-123213: Fixed xml.etree.ElementTree.Element.extend and assi…
747abc0 
…gnment to no longer hide exceptions (GH-123214) (#123258)

Co-authored-by: Bar Harel <bharel@barharel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@picnixz picnixz picnixz left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bharel @picnixz @serhiy-storchaka