6

I am learning Python and have been trying to make a deque. However, I get incorrect output and I am not sure why. My code is as follows:

p = [2, 1], [1, 1]
init_q= deque()

init_q.append(p)
for i in range(len(p)):
    for j in range(len(p[i])):
        temp = p[i][j]
        p[i][j] = 0
        init_q.append(p)
        p[i][j] = temp

while init_q:
    print init_q.pop()

In this code I take in a list, I then want to create a queue with 5 list, 4 of which have a 0 in them at different locations, the result I want is:

([2, 1], [1, 1])
([0, 1], [1, 1])
([2, 0], [1, 1])
([2, 1], [0, 1])
([2, 1], [1, 0])

However, the result I get is:

([2, 1], [1, 1])
([2, 1], [1, 1])
([2, 1], [1, 1])
([2, 1], [1, 1])
([2, 1], [1, 1])
Sjieke
  • 384
  • 1
  • 2
  • 9

3 Answers3

4

You are putting an object in the deque, then changing the object. In fact, you always put the same object into the deque, so all the deque has are references to one object p.

Ned Batchelder
  • 364,293
  • 75
  • 561
  • 662
  • And to get around this issue you would...? – Waleed Khan Jan 07 '13 at 03:30
  • 2
    Make a copy of the object - `p[:]`, or if that doesn't perform, make a `deepcopy` of the list using the `copy` module, and then append the *copy* to the list. – Volatility Jan 07 '13 at 03:32
  • Thanks I didn't know what I was looking for but I solved. `p[:]` didn't work, but `deepcopy` did. So it now looks like: `newobj = copy.deepcopy(p)` `newobj[i][j] = 0` `init_q.append(newobj)` – Sjieke Jan 07 '13 at 03:43
  • If you're getting confused by `list` mutability and identity, another way to solve this is to rewrite your code to make a new `list` in-place instead of modifying the existing one. (You can use `tuple` instead of `list` to verify that you don't accidentally mutate anything.) That doesn't always make things simpler, but sometimes it does—and either way, it always makes it easier to reason about. – abarnert Jan 07 '13 at 04:35
3

I created a visualization on Python Tutor by simplifying your code. Fiddle around and you can easily see what's happening.

A single line change to your code can fix this.

init_q.append(map(list, p))    # Initialize a new list from p's element lists

Here's the visualization by using the above change.

siddharthlatest
  • 2,237
  • 1
  • 20
  • 24
1

Following up my comment to Ned Batchelder's answer, here's how you could do the same thing immutably:

for i in range(len(p)):
    for j in range(len(p[i])):
        temprow = [0 if y==j else p[i][y] for y in range(len(p[i]))]
        temp = [temprow if x==i else p[x] for x in range(len(p))]
        init_q.append(temp)

In this case, I think the result is a lot less readable than his suggestion:

        temp = copy.deepcopy(p)
        temp[i][j] = 0
        init_q.append(temp)

As I said, sometimes it makes things simpler, sometimes less simple… But the point is that it's easier to reason about. You don't have to worry about whether multiple lists in init_q—or, worse, sub-lists within those lists—are sharing identity.

Whether the tradeoff is worth it is really a case-by-case decision, and probably different for each programmer. In this case, I wouldn't use the immutable solution, and I doubt many other (Python) programmers would. But it's worth knowing how to write it.

You also might consider writing this as a 3D list, instead of a deque of 2D lists, and then feeding it into the deque. It's obviously equivalent, but conceptually it may be simpler to think of this way:

init_q.append(p)
q = [copy.deepcopy(p) for i in range(len(p)) for j in range(len(p[i]))]
for i in range(len(p)):
    for j in range(len(p[i])):
        q[i*len(p[i])+j][i][j] = 0
init_q.extend(q)

PS, if you're doing a lot of this kind of thing, you may want to take a look at numpy. If this is your whole problem, it's not going to do you any good… but if you do anything more complicated with multi-dimensional arrays, it will.

abarnert
  • 354,177
  • 51
  • 601
  • 671