From: Serhiy Storchaka Date: Mon, 12 Jan 2026 09:49:18 +0000 (+0200) Subject: gh-142306: Improve errors for Element.remove() (GH-142308) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f3759d21dd5e6510361d7409a1df53f35ebd9a58;p=thirdparty%2FPython%2Fcpython.git gh-142306: Improve errors for Element.remove() (GH-142308) * Raise TypeError for non-element argument in the Python implementation too. * Include the repr of the elements in the ValueError error message. --- diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py index 7aa949b28191..93162f52ba03 100644 --- a/Lib/test/test_xml_etree.py +++ b/Lib/test/test_xml_etree.py @@ -370,8 +370,7 @@ class ElementTreeTest(unittest.TestCase): self.serialize_check(element, '') # 4 element.remove(subelement) self.serialize_check(element, '') # 5 - with self.assertRaisesRegex(ValueError, - r'Element\.remove\(.+\): element not found'): + with self.assertRaises(ValueError): element.remove(subelement) self.serialize_check(element, '') # 6 element[0:0] = [subelement, subelement, subelement] @@ -2758,6 +2757,17 @@ class BasicElementTest(ElementTestCase, unittest.TestCase): self.assertEqual(e2.tag, 'group') self.assertEqual(e2[0].tag, 'dogs') + def test_remove_errors(self): + e = ET.Element('tag') + with self.assertRaisesRegex(ValueError, + r" not in "): + e.remove(ET.Element('subtag')) + with self.assertRaisesRegex(TypeError, + r".*\bElement, not type"): + e.remove(ET.Element) + with self.assertRaisesRegex(TypeError, + r".*\bElement, not int"): + e.remove(1) class BadElementTest(ElementTestCase, unittest.TestCase): diff --git a/Lib/xml/etree/ElementTree.py b/Lib/xml/etree/ElementTree.py index 92f902b9a8b8..e3d81a2c4560 100644 --- a/Lib/xml/etree/ElementTree.py +++ b/Lib/xml/etree/ElementTree.py @@ -263,12 +263,15 @@ class Element: ValueError is raised if a matching element could not be found. """ - # assert iselement(element) try: self._children.remove(subelement) except ValueError: + # to align the error type with the C implementation + if isinstance(subelement, type) or not iselement(subelement): + raise TypeError('expected an Element, not %s' % + type(subelement).__name__) from None # to align the error message with the C implementation - raise ValueError("Element.remove(x): element not found") from None + raise ValueError(f"{subelement!r} not in {self!r}") from None def find(self, path, namespaces=None): """Find first matching element by tag name or path. diff --git a/Misc/NEWS.d/next/Library/2025-12-05-17-22-25.gh-issue-142306.Gj3_1m.rst b/Misc/NEWS.d/next/Library/2025-12-05-17-22-25.gh-issue-142306.Gj3_1m.rst new file mode 100644 index 000000000000..ac39490a31e7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-05-17-22-25.gh-issue-142306.Gj3_1m.rst @@ -0,0 +1,2 @@ +Improve errors for :meth:`Element.remove +`. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 22d3205e6ad3..f60a4c295e64 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -1679,8 +1679,7 @@ _elementtree_Element_remove_impl(ElementObject *self, PyObject *subelement) } if (rc == 0) { - PyErr_SetString(PyExc_ValueError, - "Element.remove(x): element not found"); + PyErr_Format(PyExc_ValueError, "%R not in %R", subelement, self); return NULL; }