4

I have a weird problem. Does anyone see anything wrong with my code?

for x in questions:
    forms.append((SectionForm(request.POST, prefix=str(x.id)),x))
    print "Appended " + str(x)
for (form, question) in forms:
    print "Testing " + str(question)
    if form.is_valid():
        forms.remove((form,question))
        print "Deleted " + str(question)
        a = form.save(commit=False)
        a.audit = audit
        a.save()                
    else:
        flag_error = True

Results in:

Appended Question 50
Appended Question 51
Appended Question 52
Testing Question 50
Deleted Question 50
Testing Question 52
Deleted Question 52

It seems to skip question 51. It gets appended to the list, but the for loop skips it. Any ideas?

jdickson
  • 696
  • 1
  • 9
  • 21

3 Answers3

12

You are modifying the contents of the object forms that you are iterating over, when you say:

forms.remove((form,question))

According to the Python documentation of the for statement, this is not safe (the emphasis is mine):

The for statement in Python differs a bit from what you may be used to in C or Pascal. Rather than always iterating over an arithmetic progression of numbers (like in Pascal), or giving the user the ability to define both the iteration step and halting condition (as C), Python’s for statement iterates over the items of any sequence (a list or a string), in the order that they appear in the sequence.

It is not safe to modify the sequence being iterated over in the loop (this can only happen for mutable sequence types, such as lists). If you need to modify the list you are iterating over (for example, to duplicate selected items) you must iterate over a copy. The slice notation makes this particularly convenient:

for x in a[:]: # make a slice copy of the entire list
...    if len(x) > 6: a.insert(0, x)

See also this paragraph from the Python Language Reference which explains exactly what is going on:

There is a subtlety when the sequence is being modified by the loop (this can only occur for mutable sequences, i.e. lists). An internal counter is used to keep track of which item is used next, and this is incremented on each iteration. When this counter has reached the length of the sequence the loop terminates. This means that if the suite deletes the current (or a previous) item from the sequence, the next item will be skipped (since it gets the index of the current item which has already been treated). Likewise, if the suite inserts an item in the sequence before the current item, the current item will be treated again the next time through the loop.

There are a lot of solutions. You can follow their advice and create a copy. Another possibility is to create a new list as a result of your second for loop, instead of modifying forms directly. The choice is up to you...

Justin Ethier
  • 131,333
  • 52
  • 229
  • 284
  • 1
    Where does it say that this is not allowed? – Marcin Mar 16 '12 at 14:50
  • So in this case I'd suggest marking questions for removal (or keeping a list of indices for questions to be deleted) and actually removing them after the loop. – egor83 Mar 16 '12 at 14:52
  • 3
    @Marcin, it says it is not safe, not that it is not allowed. Python tries to treat users as responsible adults: if you know it is not safe and you choose to go ahead then it is at your own risk, but don't come and complain when it doesn't do what you expect. – Duncan Mar 16 '12 at 14:55
  • 1
    @Duncan Please be aware that it is possible for answers to change. My comment is 10 minutes old; the answer is 4 minutes old in its current form, because JustinEthier has corrected it. In any case, the behaviour in python for this situation is not undefined, and despite what the tutorial says, it is (largely) safe and (definitely) well-defined what happens when an element is removed from a list. – Marcin Mar 16 '12 at 15:04
  • @Marcin - But is this behavior undocumented? Would we get the same results when running the code on different Python's such as Jython, IronPython, etc? – Justin Ethier Mar 16 '12 at 15:09
  • 3
    @JustinEthier The behaviour is specified here: http://docs.python.org/reference/compound_stmts.html#the-for-statement ; whether or not other implementations exhibit the same behaviour is an aspect of their conformance to the specification. – Marcin Mar 16 '12 at 15:20
  • @Marcin - Awesome, I just added that reference to the answer. Thanks! – Justin Ethier Mar 16 '12 at 15:24
  • @JustinEthier Feel free to show some appreciation through the vote system. – Marcin Mar 16 '12 at 15:29
  • Another, often better, approach is to build a new list that consists of the modified result, and then switch out the old list for the new one. In particular, if the list primarily works by removing elements, then you build a new list consisting of the ones you **don't** want to remove. It's often very possible, and idiomatic, to condense this process into a list comprehension. – Karl Knechtel Mar 16 '12 at 18:21
4

You are removing objects from forms while iterating over it. This is supposed to lead to the behaviour you are seeing ( http://docs.python.org/reference/compound_stmts.html#the-for-statement ).

The solution is to either iterate over a copy of that list, or add the forms for removal to a separate collection, then perform the removal afterwards.

Marcin
  • 48,559
  • 18
  • 128
  • 201
2

Using the remove method on forms (which I assume is a list) changes the size of the list. So think of it this way

[ 50, 51, 52 ]

Is your initial list, and you ask for the first item. You then remove that item from the list, so it looks like

[51, 52]

But now you ask for the second item, so you get 52.

mklauber
  • 1,126
  • 10
  • 19