Conway's Game of Life in Python, saving to an image

I have attempted to make conways game of life in Python, and then save the output into a picture.

Is there any way to make this more efficient?
Is there anything wrong with the logic? (Most of the pictures don't look quite correct)

Other?

import PIL.Image, random

WIDTH = 1366
HEIGHT = 768
ROUNDS = 10

DEAD = (0, 0, 0)
ALIVE = (0, 64, 255)

print("Creating image")
img = PIL.Image.new("RGB", (WIDTH, HEIGHT))
data = img.load()

print("Creating grid")
grid = []
for y in range(HEIGHT):
    grid.append([])
    for x in range(WIDTH):
        grid[y].append(random.randint(0, 1))

for i in range(ROUNDS):
    print("Starting round", i + 1, "of", ROUNDS)
    for y in range(HEIGHT):
        for x in range(WIDTH):
            n = 0
            for y2 in range(-1, 2):
                for x2 in range(- 1, 2):
                    if x2 != 0 and y2 != 0 and grid[(y + y2) % HEIGHT][(x + x2) % WIDTH] == 1:
                        n += 1
            if n  3:
                grid[y][x] = 0
            elif grid[y][x] == 1 and n > 1 and n < 4:
                grid[y][x] = 1
            elif grid[y][x] == 0 and n == 3:
                grid[y][x] = 1

print("Rendering image")
for y in range(HEIGHT):
    for x in range(WIDTH):
        if grid[y][x] == 1:
            data[x, y] = ALIVE
        else:
            data[x, y] = DEAD

print("Saving image")
img.save("gofl.png")

Style

Your code reads nice, you made a good job following PEP8, except for the 2 imports on the same line and a line exceeding 80 characters.

Other than that you’re missing Python's higher-level constructs and your logic is slightly off.

List comprehension

Creating the grid

Building a list using append in a for loop is considered an anti-pattern in Python. You should use a list-comprehension instead:

# No
l = []
for i in range(X):
    l.append(get_data(i))

# Yes
l = [get_data(i) for i in range(X)]

In case you want a list of lists (as you do for grid) you can easily combine them:

grid = [[random.randint(0, 1) for _ in range(WIDTH)] for _ in range(HEIGHT)]

Note the use of _ to show that we won't use the iteration variable and that we are only interested in repeating the same operation a certain amount of time.

Counting neighbours

You can use almost the same syntax than list-comprehensions to create generator expressions: simply change the surrounding brackets into parenthesis. The difference being that generator expressions won't produce the results at once and store them in memory. Instead, they will be generated as needed but you’ll be able to read them only once.

This is perfect when you don't need to store an intermediate list just to iterate once over it. For instance, if we were to feed a list into the sum function, and that list was built using a list-comprehension, it's probably better to use a generator expression instead.

And that's what is better to count neighbours of a given cell: use the sum builtin with a generator expression. It will also help removing that annoying bug of yours in that big if that tries to remove the current cell out of the count. Instead you’re only counting the neighbours that are in the corner of the surrounding square (I means, those which have neither x2 == 0 nor y2 == 0.) Instead of trying to come up with broken conditions, you’d better of adding the 9 cells and then removing the value of the current one.

So, how do we turn these for loops into generator expressions:

# Assuming NEIGHBOURS = range(-1, 2)
n = sum(
    sum(row[(x+x2) % WIDTH] for x2 in NEIGHBOURS)
    for row in grid[(y+y2) % HEIGHT] for y2 in NEIGHBOURS
)

Here I used a subexpression within the main expression, pretty much like I did with the two for loops that created grid. Setting NEIGHBOURS = range(-1, 2) is also a good idea as it adds meaning and removes duplication.

To get the actual number of neighbours, you just need to substract grid[y][x] from that.

Iterations

In Python we don't like to iterate over a sequence using indexes. We have constrcuts that allow us to directly iterate over elements and it’s faster to do so. But sometimes, like here, you also need the indexes (to update the original data-structure, for instance.) In such case, it's prefered to use enumerate instead of range(len):

for y, row in enumerate(grid):
    for x, cell in enumerate(row):
        data[x, y] = ALIVE if cell else DEAD

Same logic for each ROUND.

You can also note the use of the ternary operator to simplify the logic a bit. Speaking of that…

Logic simplification

Using the ternary was a first step but I think that the following construct reads good as well:

DEAD_OR_ALIVE = (
    (0, 0, 0),
    (0, 64, 255),
)

...

for y, row in enumerate(grid):
    for x, cell in enumerate(row):
        data[x, y] = DEAD_OR_ALIVE[cell]

It might also be a little bit faster (even though I didn't time it).

Handling birth and death of cells can also be simplified: a neighbour's count of 3 will either keep the current cell alive or create a new one in the current slot: grid[y][x] = 1 in both cases; a count of 2 will leave the current slot untouched; and anything else will kill the current cell:

for y, row in enumerate(grid):
    for x, cell in enumerate(row):
        n = ...
        if n == 3:
            grid[y][x] = 1
        elif n == 2:
            pass
        else:
            grid[y][x] = 0

Logic management

One thing that I don't quit get is why you create the image at the beginning of the program only to populate it after all computation is done. You could create it near the end and have related bits of logic near of each other.

Better, you could wrap each logical piece of stuff into it's own function to ease testing and reusability (you can import you file in an interactive shell and call your functions from there with varying parameters.) Doing that you could simplify your constants by using default values for parameters.

You can also improve the overall layout by adding an if __name__ == '__main__' clause to wrap the remaining of your top-level code into.

Proposed improvements

```
import PIL.Image
import random

WIDTH = 1366
HEIGHT = 768
ROUNDS = 10
NEIGHBOURS = range(-1, 2)

def random_grid(width=WIDTH, height=HEIGHT):
return [
[random.randint(0, 1) for _ in range(width)]
for _ in range(height)

StackExchange Code Review Q#125292, answer score: 2