# Count consecutive ones in a binary list

There is a list consisting of zeroes and ones. I want to find out the length of the longest streak of ones. Is there a better solution?

def consecutive_one(data):
one_list = []
size = 0
for num in data:
if num == 1:
one_list.append(num)
elif num == 0 and size < len(one_list):
size = len(one_list)
one_list = []

return size

if __name__ == '__main__':
data = [0, 1, 0, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0]
print(consecutive_one(data))


## 3 Answers

Both your and janos' implementations are broken:

data = [1, 0] * 10000
consecutive_one(data)
#>>> 140


This is because you don't always reset after seeing a 0. Going from janos', you should have

longest = 0
current = 0
for num in data:
if num == 1:
current += 1
else:
longest = max(longest, current)
current = 0

return max(longest, current)


and equivalent for the original.

You'll find that this functionality is largely provided by itertools.groupby, though:

from itertools import groupby

def len_iter(items):
return sum(1 for _ in items)

def consecutive_one(data):
return max(len_iter(run) for val, run in groupby(data) if val)


### You have a bug

If the last value is a 1, and it is the end of the longest consecutive sequence, it won't be taken into account. The fix is to change the return statement to this:

return max(size, len(one_list))


### Unnecessary condition

If you know your input only contains 0 and 1 values, then you can simplify this condition:

if num == 1:
# ...
elif num == 0 and size < len(one_list):
# ...


By dropping the num == 0:

if num == 1:
# ...
elif size < len(one_list):
# ...


But note that this is not good enough, as there's still a bug hiding there as @veedrac explains in his answer, instead of an elif, this should be rewritten using an else.

### Improving storage efficiency

There's no need to store the 1s as you count them. You can just keep the count in a variable.

### Testing

Instead of running your function using test data, give a try to doctests, like this:

def consecutive_one(data):
"""
>>> consecutive_one([0, 1, 0, 1, 1, 0])
2
>>> consecutive_one([0, 1, 0, 1, 1, 1])
3
>>> consecutive_one([0, 1] * 10)
1
"""
# ... the implementation ...


To run all doctests within a file, run python -m doctest yourfile.py. When all tests pass, there is no output. When something fails you will get a detailed report. This is an excellent way to test your implementation, and also to document usage with examples and expected outputs.

• Just to be more precise, he doesn't have to know that the list contains only 0 and 1. The purpose is to count every 1, so it's not really relevant if it's 0 or another character, as long as it's not 1. – ChatterOne Aug 12 '16 at 20:48

You have a bug on elif statement size < len(one_list):

if __name__ == '__main__':
n = int(input())

binary = [int(x) for x in bin(n)[2:]]

one_list = []
size = 0

for num in binary:
if num == 1:
one_list.append(num)

if size < len(one_list):
size = len(one_list)

elif num == 0 :
one_list.clear()

print(size)

• (Welcome to Code Review!) (While valid, this has been stated before.) – greybeard Dec 14 '18 at 21:27