bpo-34160: Preserve user specified order of Element attributes#10163
bpo-34160: Preserve user specified order of Element attributes#10163rhettinger merged 2 commits intopython:masterfrom
Conversation
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Remove sorting also from other output methods (html, text). Add a What's New entry (in "Porting to 3.8").
Would you mind to remove sorting also from minidom output in this PR?
Lib/xml/etree/ElementTree.py
Outdated
| _escape_attrib(v) | ||
| )) | ||
| for k, v in sorted(items): # lexical order | ||
| for k, v in items: # lexical order |
There was a problem hiding this comment.
I don't think this warrants a whatsnew entry.
There was a problem hiding this comment.
If other modules are to be changed as well (minidom, html modules, etc), it will need to be done in another PR. I don't have the spare time for those right now. Perhaps the other contributor on the tracker issue will continue to show an interest.
There was a problem hiding this comment.
At least remove sorting in other output methods in this module. And add tests for them.
Lib/test/test_xml_etree.py
Outdated
| root = ET.Element('cirriculum', status='public', company='example') | ||
| tree = ET.ElementTree(root) | ||
| try: | ||
| fo = open(support.TESTFN, "wb") |
There was a problem hiding this comment.
I would suggest to use addCleanup(support.unlink, support.TESTFN) and with for working with files, or use io.StringIO. But it is better to use the serialize() helper.
There was a problem hiding this comment.
Not sure what serialize() is. Switched from the traditional test support code to a context manager that does make it a little nicer.
There was a problem hiding this comment.
serialize() is a helper in this file which writes an XML to string/bytes/StringIO/BytesIO. It will allow you to use a single line instead of three or four lines.
self.assertEqual(serialize(tree),
'<cirriculum status="public" company="example" />')
4 participants
Minidom should also be fixed but that can be done in a separate PR.
https://bugs.python.org/issue34160