Repository navigation
xml.etree.ElementTree: Element.append doesn't type-check its argument #57991
Description
| BPO | 13782 |
|---|---|
| Nosy | @terryjreedy, @florentx |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2012-03-23.12:26:57.987>
created_at = <Date 2012-01-14.00:54:12.734>
labels = ['expert-XML', 'type-bug', 'library']
title = "xml.etree.ElementTree: Element.append doesn't type-check its argument"
updated_at = <Date 2012-03-23.12:27:11.787>
user = 'https://bugs.python.org/sjmachin'bugs.python.org fields:
activity = <Date 2012-03-23.12:27:11.787>
actor = 'eli.bendersky'
assignee = 'none'
closed = True
closed_date = <Date 2012-03-23.12:26:57.987>
closer = 'eli.bendersky'
components = ['Library (Lib)', 'XML']
creation = <Date 2012-01-14.00:54:12.734>
creator = 'sjmachin'
dependencies = []
files = ['24898']
hgrepos = []
issue_num = 13782
keywords = ['patch']
message_count = 11.0
messages = ['151210', '151233', '151708', '152989', '153008', '153024', '153025', '156136', '156141', '156650', '156651']
nosy_count = 8.0
nosy_names = ['effbot', 'terry.reedy', 'sjmachin', 'eli.bendersky', 'flox', 'santoso.wijaya', 'tshepang', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13782'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']
Activity
sjmachin commentedon Jan 14, 2012
2.7 and 3.2 produce no complaint at all.
2.6 and 3.1 produce an AssertionError.
However cElementTree in all 4 versions produces a TypeError.
Please fix 2.7 and 3.2 ElementTree to produce a TypeError.
santosowijaya commentedon Jan 14, 2012
This does not only affect
append, but also,insert,extend, etc.In fact, the list-like operations on Element are merely forwarded to its
_children(a list) field. Should we really type check these methods' arguments each? Doesn't strike as Pythonic to me...OTOH, in cElementTree, by virtue of C-API's tuple unpacking convention, the argument's type is always strictly checked prior to accessing the children list.
terryjreedy commentedon Jan 20, 2012
Confirmed with on 3.2.2 Win7 for all three methods.
Docs say .append should raise AssertionError (see below).
However, that is unusual and TypeError is normal practice.
I am not sure what to do.
Our docs say: '''
append(subelement)
Adds the element subelement to the end of this elements internal list of subelements.
This implies to me that append and insert should do the same.
In fact, the current code has the assertion checks but they are commented out in all methods.
From
http://effbot.org/zone/pythondoc-elementtree-ElementTree.htm
"append ... Raises AssertionError:
If a sequence member is not a valid object."
Ditto for .insert
Florent: why are all the assertion checks commented out in
Changeset: 59511 (831e30c0fd73) Issue bpo-6472:
The xml.etree package is updated to ElementTree 1.3.
(March 2010)
Is the change in Fredrik's code, in spite of his docs?
florentx commentedon Feb 9, 2012
Actuallly, the assertions were commented in the upstream repository.
https://bitbucket.org/effbot/et-2009-provolone/src/9e9e7f2710aa/elementtree/CHANGES#cl-46
It looks more useful to me to have a consistent behavior between C and Python version.
elibendersky commentedon Feb 10, 2012
I agree, especially is we now consider to expose the C API by default, leaving the Python API as a fallback.
terryjreedy commentedon Feb 10, 2012
Option 1 is to fix the regression in the 2.7/3.2 Python versions and change the C version to conform to the doc and older Python versions., even though it is the wrong error, and even though Python asserts disappear with -O (which is why it is the wrong error).
Option 2 is to leave cET alone with the correct error and fix 2.7/3.2 and the docs to conform.
Option 3 is to leave cET alone and leave 2.7/3.2 alone as done deals, even if bad. Change 3.3 Python and docs to TypeError. We would need a version-changed note, but perhaps should mention the 3.2 behavior so as to not give a mis-impression.
I think I favor 3, but after presenting the trilemma on pydev.
elibendersky commentedon Feb 10, 2012
Terry, I agree with 3. The "regression" here is insignificant enough in my view to warrant mucking with already released versions.
elibendersky commentedon Mar 17, 2012
Link to related python-dev discussion: http://mail.python.org/pipermail/python-dev/2012-March/117715.html
elibendersky commentedon Mar 17, 2012
Attaching a patch that sets the record straight in 3.3 - the 3 methods will raise TypeError, in both C and Python implementations. Also adds a test that verifies this, and updating the documentation.
Note that in the C implementation extend wasn't actually type-checking its argument as well, so I added it for consistency. I don't believe this will be a performance problem. On the contrary, there are some type checking calls inside the implementation that can probably be removed now that we make sure only subclasses of Element get in as children. I'll look into this later.
python-dev commentedon Mar 23, 2012
New changeset 75268a773455 by Eli Bendersky in branch 'default':
Issue bpo-13782: streamline argument type-checking in ET.Element
http://hg.python.org/cpython/rev/75268a773455
elibendersky commentedon Mar 23, 2012
Closing the issue.
3.3 was fixed, and there was no concensus regarding 2.7/3.2 in the list, so status-quo as usual.