X Tutup
Skip to content

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

Merged
serhiy-storchaka merged 9 commits intopython:mainfrom
bharel:gh-123213
Aug 23, 2024
Merged

gh-123213: Fixed xml.etree.ElementTree.Element.extend and assignment to no longer hide exceptions#123214
serhiy-storchaka merged 9 commits intopython:mainfrom
bharel:gh-123213

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
@miss-islington-app
Copy link

miss-islington-app bot commented Aug 23, 2024

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
X Tutup