?

Log in

In which I admit to making a stupid mistake in Python - Will Woods, Fedora Testing Guy [entries|archive|friends|userinfo]
Will Woods, Fedora Testing Guy

[ website | https://ohjeezlinux.wordpress.com/ ]
[ userinfo | livejournal userinfo ]
[ archive | journal archive ]

In which I admit to making a stupid mistake in Python [Feb. 24th, 2010|01:44 pm]
Will Woods, Fedora Testing Guy
So yesterday we found a bug in RATS (Rawhide Acceptance Test Suite - the scripts we use for testing Rawhide and other Fedora candidate trees). The test found no problems with the Fedora 13 release candidate tree we'd just made, but when we tried to install it, the installer died because the kernel had missing dependencies. Huh? The tests are supposed to check that!

We checked the logs, and the test script hadn't even checked the kernel package. But 'kernel' was definitely in the list of packages that were supposed to be checked - and it was still in the list after we got through the loop that checked all the packages. But it never got tested. What gives?

The cause turned out to be an embarrassing mistake on my part. Consider the following python snippet:
meats = ('bacon', 'pork', 'beef')

input = ['one', 'two', 'pork', 'three', 'four']

for n in input:
    if n in meats:
        input.remove(n)
    else:
        print n

What would you expect to see as output? Probably 'one, two, three, four', right? Instead, you'll get:
one
two
four

Shortening a list while you're iterating over it turns out to be a bad idea. You can append to the list just fine, and the loop will happily iterate over the new items you added once it gets to the end of the list. But if you remove an item, bad things happen. Here's what happens:

When the loop is processing 'pork', it's processing the third item in the list. When you remove 'pork', the list gets shifted up, so now 'three' becomes the third item in the list.

Then we hit the end of the loop and move to the fourth item in the list - without ever processing 'three'!

So it turns RATS was removing the package before 'kernel' in the package list - which was the right thing to do - but that caused us to accidentally skip 'kernel', leading to the false positive result from the test.

Long story short: Never remove items from a list while you're looping over it.
linkReply

Comments:
[User Picture]From: jeckersb
2010-02-24 09:39 pm (UTC)

easiest fix

If you slice the list you iterate over to make a copy it works as expected.

becomes...

for n in input[:]:
(Reply) (Thread)
[User Picture]From: qa_rockstar
2010-02-24 10:05 pm (UTC)

Re: easiest fix

Oh, that's a good solution, at least for the example given. Unfortunately the actual code in question was a little more complicated - it also sometimes needed to add items to the list as it went along. Using an anonymous copy of the list would mean that we wouldn't have anywhere to add the new items, so it wouldn't have worked in this case.

The solution I ended up using was basically this:
meats = ('bacon', 'pork', 'beef')

input = ['one', 'two', 'pork', 'three', 'four']

badlist=[]
for n in input:
    if n in meats:
        badlist.append(n)
    else:
        print n
        # we might also add items to the initial list here, like so:
        if n == 'four':
            input.append('five')

for n in badlist:
    input.remove(n)

Which will correctly print 'one two three four five' and have exactly those five items in the list when you're done. Yay!
(Reply) (Parent) (Thread)
[User Picture]From: pfrields.id.fedoraproject.org
2010-02-24 10:48 pm (UTC)

A moment of duh for you...

Hours saved for poor schmos like me. Thanks for posting this, Will. :-)
(Reply) (Thread)
[User Picture]From: orangejulius
2010-02-25 03:34 pm (UTC)
This particular quirk was maddening when I first learned python, so much so that I gave up on trying to modify lists at all while iterating through them (and also that I switched to another language when I had a need to do it).
(Reply) (Thread)