Repository navigation
ElementTree not preserving attribute order #78341
Description
Activity
Starting with Python3.6, the order of keyword arguments has been guaranteed. Something in ElementTree is not respecting that order.
$ python3.7 Python 3.7.0 (v3.7.0:1bf9cc5093, Jun 26 2018, 23:26:24) [Clang 6.0 (clang-600.0.57)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> from xml.etree.ElementTree import Element, dump >>> dump(Element('cirriculum', status='public', company='example')) <cirriculum company="example" status="public" /> >>> # ^-----------------^-------------------- These are swappedAttributes are sorted by name when converted an element to the text representation for making it stable. Changing this may break existing software.
I think it makes sense now to add an optional keyword-only parameter sort_attrs (True by default) for functions tostring() and tostringlist() and method ElementTree.write(). If it is False, attributes will keep the original order.
Since dump() is used for debugging only, it could change the behavior by default, even in maintained releases. But currently it just uses ElementTree.write(), which should sort attributes by default for compatibility.
At least for lxml, attributes were never specified to have a sorted order (although attribute dicts are sorted on *input*, to give a *predictable* order as in ET), and the tutorial says: "Attributes are just unordered name-value pairs".
However, I still think that Serhiy is right: this change would break code, and in particular test code that compares XML output. Having to deal with two different "correct" serialisations in tests depending on the Python version is annoying at best. But OTOH, comparing XML output should always be based on C14N and not on an arbitrary serialisation. XML C14N serialisation was specifically designed to provide a deterministic output byte sequence, regardless of how the XML document was created. (This is also used for cryptographic fingerprinting.)
It is interesting that ET sorts the attributes on output. lxml does it only on API *input*, because the underlying tree model is order preserving. Parsed attributes are always written in the original document order, followed by the attributes set through the API in the order in which they were set.
For lxml, a serialisation flag is not going to help, because the serialisation order is always deterministic from the time where an attribute is added to the tree. Given that dicts are order preserving in Python now, ET could follow this path as well. The attributes are already in parse order and could easily be serialised that way, and attributes that were added through the API would end up being serialised last, also preserving the order. This would leave lxml and ET with the same serialisation order, which seems attractive.
In short, I like this change, it's just difficult to see a way how this could preserve full backwards compatibility. And it's difficult to say how important backwards compatibility really is here.
This doesn't strike me as a bug, or even clearly being a potential improvement.
If this about round-trip parsing/serializing not preserving order, that would be significant. As it is, if this is only intended as a debugging tool, why change it?
Is there a concrete reason to change this and break backwards compatibility? If not I suggest we close this as "won't fix".
Consider this as a feature request. It is perfectly reasonable for a user to want to generate specific XML output and to want to control the order that the attributes are listed. It is perfectly reasonable for the API to preserve the order that the user specified rather than change it into some other order that they either don't expect or can't control. Starting in Python 3.6, the language guarantees that the order of keyword arguments is preserved. Knowing that, it will be natural for users to start viewing as buggy APIs that discard that order when everything else in the eco-system respects the ordering.
When I teach ElementTree in Python courses, students notice this behavior and ask why the order was swapped. My only answer is that we didn't used to have a way to control it, so the input order was ignored (i.e. a reason that no longer makes sense).
Thanks for the clarification, Raymond.
I've spent a few minutes searching for uses of dump(), and have only come up with uses for debugging or printing helpful info in command line tools. So, it seems that changing this as suggested wouldn't break much existing code, since I couldn't easily find even one such example.
This is an easy issue and I left it for beginning contributors. The PR should include the following changes:
-
Add an optional sort_attrs parameter (True by default) in the write() method.
-
Make the dump() function passing sort_attrs=False to write().
-
Add tests for write() (with sort_attrs=False and sort_attrs=True) and dump() and/or modify existing tests.
-
Document change (in the module docs, in What's New, news entry).
-
69 remaining items
Thanks to the discussion that rhettinger triggered on python-dev, it seems that there is a majority accordance for considering the previous ordering "undefined but deterministic" rather than "sorted", and for making the change from "arbitrarily sorted" to "user defined" in Py3.8. That matches the current status after the merge of the four pull requests, for both ElementTree and MiniDOM.
There also seems to be a large fraction of participants that would like to see a stdlib way to safely/correctly/easily compare XML output. IMHO, that is covered by bpo-13611, adding a C14N module to ElementTree. There is also a (Py2) C14N implementation for minidom, from the now unmaintained PyXML package¹, which, according to the license, is already distributed under Python copyright. I will try to bring at least the ET module in shape for integration before 3.8 beta1.
During the discussions, several ways were shown to achieve deterministic behaviour, also across Python releases. My proposal is therefore to *not* add a new "sort_attrs" option to the API, with the reasoning that it is a) not required for any of the use cases, b) not maintaining compatibility without code changes (neither backwards nor forward), and c) not what most people actually want who strive for deterministic output. Instead, I think we should concentrate on providing and documenting better ways to compare XML output.
¹ https://github.com/actmd/PyXML/blob/97f144152878a67cd95dd229fe72e86f19bce706/xml/dom/ext/c14n.py
Stefan, as the module maintainer, you get to make the final decision on this. What you've proposed makes sense in light of the prior discussion and the python-dev discussion. Unless you think something new will come along to change your mind, just mark as closed/fixed, then continue work on c14n in another tracker issue.
This is done now. Thanks everyone who helped in discussing and implementing this change.
I will leave Serhiy's last PR (adding the "sort_attrs" flag option) open for a while until I'm sure we have a better solution for comparing XML in 3.8, at which point I would reject it.
FYI pungi project is also broken by this change and it blocks Fedora Rawhide to upgrade to Python 3.8:
https://bugzilla.redhat.com/show_bug.cgi?id=1698514I rejected the (now conflicting) PR that adds a sorting option.
I also sent Victor a tentative (and trivial) patch for the pungi package.Other examples of projects broken by this change:
- python-glyphsLib: https://bugzilla.redhat.com/show_bug.cgi?id=1705391
- pcs: https://bugzilla.redhat.com/show_bug.cgi?id=1705475
What's New In Python 3.8 only briefly mention this change:
"The writexml(), toxml() and toprettyxml() methods of the xml.dom.minidom module, and xml.etree now preserve the attribute order specified by the user. (Contributed by Diego Rojas and Raymond Hettinger in bpo-34160.)"
https://docs.python.org/dev/whatsnew/3.8.html#changes-in-the-python-apiWould it be possible to suggest solutions for this backward incompatible change? Like https://bugs.python.org/issue34160#msg338102 recipe. Maybe we should even put it in the XML documentation, somewhere.
dfrojas commented
on Jul 15, 2019 dfrojasmannequinMannequinVictor, you mean place again this code? https://github.com/python/cpython/pull/10163/files#diff-d5a064acb6ae44dcb7e01fee148c733dR926
And the recipe proposed in https://bugs.python.org/issue34160#msg338102 place it as a method of ElementTree or just as a helper function?
I created a PR that adds a couple of paragraphs to the documentation. Comments welcome.
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:
bugs.python.org fields: