Skip to content

Commit f3759d2

Browse files
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.
1 parent dbd10a6 commit f3759d2

File tree

4 files changed

+20
-6
lines changed

4 files changed

+20
-6
lines changed

Lib/test/test_xml_etree.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,7 @@ def test_simpleops(self):
370370
self.serialize_check(element, '<tag key="value"><subtag /></tag>') # 4
371371
element.remove(subelement)
372372
self.serialize_check(element, '<tag key="value" />') # 5
373-
with self.assertRaisesRegex(ValueError,
374-
r'Element\.remove\(.+\): element not found'):
373+
with self.assertRaises(ValueError):
375374
element.remove(subelement)
376375
self.serialize_check(element, '<tag key="value" />') # 6
377376
element[0:0] = [subelement, subelement, subelement]
@@ -2758,6 +2757,17 @@ def test_pickle_issue18997(self):
27582757
self.assertEqual(e2.tag, 'group')
27592758
self.assertEqual(e2[0].tag, 'dogs')
27602759

2760+
def test_remove_errors(self):
2761+
e = ET.Element('tag')
2762+
with self.assertRaisesRegex(ValueError,
2763+
r"<Element 'subtag'.*> not in <Element 'tag'.*>"):
2764+
e.remove(ET.Element('subtag'))
2765+
with self.assertRaisesRegex(TypeError,
2766+
r".*\bElement, not type"):
2767+
e.remove(ET.Element)
2768+
with self.assertRaisesRegex(TypeError,
2769+
r".*\bElement, not int"):
2770+
e.remove(1)
27612771

27622772
class BadElementTest(ElementTestCase, unittest.TestCase):
27632773

Lib/xml/etree/ElementTree.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,15 @@ def remove(self, subelement):
263263
ValueError is raised if a matching element could not be found.
264264
265265
"""
266-
# assert iselement(element)
267266
try:
268267
self._children.remove(subelement)
269268
except ValueError:
269+
# to align the error type with the C implementation
270+
if isinstance(subelement, type) or not iselement(subelement):
271+
raise TypeError('expected an Element, not %s' %
272+
type(subelement).__name__) from None
270273
# to align the error message with the C implementation
271-
raise ValueError("Element.remove(x): element not found") from None
274+
raise ValueError(f"{subelement!r} not in {self!r}") from None
272275

273276
def find(self, path, namespaces=None):
274277
"""Find first matching element by tag name or path.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Improve errors for :meth:`Element.remove
2+
<xml.etree.ElementTree.Element.remove>`.

Modules/_elementtree.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,8 +1679,7 @@ _elementtree_Element_remove_impl(ElementObject *self, PyObject *subelement)
16791679
}
16801680

16811681
if (rc == 0) {
1682-
PyErr_SetString(PyExc_ValueError,
1683-
"Element.remove(x): element not found");
1682+
PyErr_Format(PyExc_ValueError, "%R not in %R", subelement, self);
16841683
return NULL;
16851684
}
16861685

0 commit comments

Comments
 (0)