2

I'm writing a function to move 0's to the end of a list, but when a False value is in the list, it converts it to 0 and moves it to the end as well.

I've tried many variations of if statements including:

  1. if i == 0

  2. if type(i) != bool and i == 0

  3. if str(i) == '0'

def move_zeros(array):
    for i in array:
        if str(i) == '0' and str(i) != 'False':
            array.remove(i)
            array.append(0)
    return array

In case of [0,1,None,2,False,1,0], I expected the function to return [1,None,2,False,1,0,0], but it returned [1,None,2,1,0,0,0] instead.

khelwood
  • 55,782
  • 14
  • 81
  • 108
hubvoy
  • 31
  • 1
  • 6

2 Answers2

2

You have two issues here. First, in Python, bool is a subclass of int and False == 0, so to tell apart 0 from False you have to test on your object's type. Also, modifying a list in place while iterating over it is a sure recipe for "off" errors.

Here's a plain stupid simple implementation that works:

def move_zeros(seq):
    head, tail = [], []
    for v in seq:
        if isinstance(v, int) and not isinstance(v, bool) and v == 0:
            tail.append(v)
        else:
            head.append(v)
    return head + tail
bruno desthuilliers
  • 75,974
  • 6
  • 88
  • 118
  • Thank you, I also added or isinstance(v,float) for possible 0.0 values – hubvoy May 27 '19 at 12:45
  • glad I could help - and feel free to upvote my answer then. You may also want to upvote and accept leporello's answer which is more accurate than mine – bruno desthuilliers May 27 '19 at 12:48
  • I'm pretty new to stack, literally signed up an hour ago so I upvoted both yours and leporello's answer but i think it won't show when i have less than 15 reputation – hubvoy May 27 '19 at 12:51
  • @hubvoy thanks for both of us 8-) - and please do not forget to _accept_ leporello's answer which deserves to be marked as the one correct answer here (remember that SO is about building a technical knowledge base so accepting the best solution is important for future readers). – bruno desthuilliers May 27 '19 at 13:48
  • ok accepting the answer is just ticking the thingy under the vote counter right? – hubvoy May 27 '19 at 14:08
1

There are a couple of gotchas here. One that you already understood is that there is not trivial way to discriminate between 0 and False. However, there are two other points:

  1. Modifying an iterable (list, dict, etc.) that you are iterating over is dangerous. You should avoid doing that (unless you cannot do otherwise, e.g. with very large amounts of data that you cannot afford to copy in memory).
  2. array.remove() does not do what you think it does. It removes from array the first element that is == to its argument, which may or may not be in the same position as the element in the iteration loop. That is responsible for transforming your False into a 0.

With that in mind, here's a working version:

def is_integer_zero(x):
    # The cleanest way I found, but you could argue for other options
    return x == 0 and not isinstance(x, bool)

def move_zeros(array):
    left_part = []
    right_part = []

    for i in array:
        if is_integer_zero(i):
            right_part.append(i)
        else:
            left_part.append(i)

    return left_part + right_part


l = [0, 1, None, 2, False, 1, 0]

print(move_zeros(l))  # [1, None, 2, False, 1, 0, 0]

right_part consists entirely of zeroes, so you could simplify the code / improve performance quite a bit, but that version has the advantage that you can use other conditions to choose what to right-shift.

Leporello
  • 638
  • 4
  • 12
  • if you want to make it more generic, consider passing the predicate function (here `is_integer_zero`) as a callback. Totally over the top for the OP's problem obviously but since you mentionned "using other conditions"... ;-) – bruno desthuilliers May 27 '19 at 13:50