5
\$\begingroup\$

I'm parsing an XML file that is structured like this:

<root>
  <entry>
    <elem1> key </elem1>
    <elem2> value </elem2>
  </entry>
  <entry>
  ....
  </entry>
</root>

</root>

My first code was using .parse() and .getRoot() but I had memory issues (1.5G memory use for 350M file). I solved my problem using an iterator and .clear() after reading this paper.

import xml.etree.ElementTree

myDict = {}

xmlIterator = xml.etree.ElementTree.iterparse('file.xml')

while True:
    try:
        _ , elem1 = next(xmlIterator)
        _ , elem2 = next(xmlIterator)
        _ , _     = next(xmlIterator)

    except StopIteration:
        break

    myDict[elem1.text]=elem2

    elem1.clear()
    elem2.clear()
    _.clear()

The issue is I need to access several children at the same time:

  • The first one is my key <elem1>
  • The second one is the value I need <elem2>
  • The third is not relevant <entry>

I would like to know if this is a good practice, or if there is a better way to do that (better performance or readability).

I also tried to follow the "Easier to ask for forgiveness than permission" principle with the exception block for ending the loop, but I'm not sure about multiple statements in the same exception block.

In fact, I also don't understand why the third element I get with next() is <entry>.

\$\endgroup\$
3
\$\begingroup\$

Looping using while True:, item = next(iterable), and except StopIteration: break is very unnatural. It is much more conventional to write for item in iterable:.

My main concern, though, is that your code is very fragile. It assumes that you will always receive the event ('end', elem1), followed by ('end', elem2), followed by ('end', entry), over and over again. I would say that the code works by coincidence, rather than by design. If there is ever, say, an unexpected <remark> element somewhere, then everything gets thrown off. If the document ever changes to

<entry>
    <elem2>value</elem2>
    <elem1>key</elem1>
</entry>

then it would also behave in an unintuitive way. Since XML is a verbose file format that explicitly describes its own structure, you should actually look at the tags instead of blindly assuming a certain structure.

Here's how I would write it:

import xml.etree.ElementTree as ET

my_dict = {}
for event, elem in ET.iterparse('file.xml'):
    assert event == 'end'
    if elem.tag == 'elem1':
        key = elem.text
    elif elem.tag == 'elem2':
        value = elem.text
    elif elem.tag == 'entry':
        my_dict[key] = value
        key = value = None
    elem.clear()

This works by temporarily keeping track of any keys and values that it sees, then actually adding the key:value pair to the dictionary when a </entry> tag is encountered.

\$\endgroup\$
4
\$\begingroup\$

This isn't the answer you want. But, _ is most commonly the discard variable. You use it when you don't want to use the variable, but you need to specify a variable. Take:

for _ in range(10):
    print('hi')

We don't want the output from the range, we just want to print 'hi' ten times. And so you should not use this for the variable you use .clear() on.

I'd use zip rather than the try-except. Sure there's Look Before You Leap (LBYL) and Easier to Ask for Forgiveness than Permission (EAFP). But you don't need to use either here. Also, by using zip you can change your while loop to a for loop too.

And so you should be able to get something like:

import xml.etree.ElementTree

elements = {}
xml_iterator = xml.etree.ElementTree.iterparse('file.xml')

for (_, elem1), (_, elem2), (_, elem3) in zip(xml_iterator, xml_iterator, xml_iterator):
    elements[elem1.text] = elem2
    elem1.clear()
    elem2.clear()
    elem3.clear()
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot. This code works just fine, and I find it much more elegant and readable. I'll test memory use and give a feedback. \$\endgroup\$ – mxdsp Nov 25 '16 at 13:50
  • \$\begingroup\$ I'm not familiar with performance tests, so I can't tell if there is any improvements, but memory use is exactly the same. \$\endgroup\$ – mxdsp Nov 25 '16 at 14:11
  • \$\begingroup\$ @mxdsp I didn't intend to increase the performance or reduce memory. Which is why I said "This isn't the answer you want." It was alternate ways to improve your code. \$\endgroup\$ – Peilonrayz Nov 25 '16 at 14:24
  • 1
    \$\begingroup\$ And you did well. You actually answer to the " better readability" part of my question. Thanks again. I was just giving feedback for anyone interested in your answer. I was concerned about zip(iterator*n) costing more memory than n * next(), it's not the case. \$\endgroup\$ – mxdsp Nov 25 '16 at 15:44
  • \$\begingroup\$ @mxdsp Ah, good idea, my apologies, :) \$\endgroup\$ – Peilonrayz Nov 25 '16 at 15:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.