Python Sudoku GUI

This code implements a data structure for representing a Sudoku board, a very simple algorithm for setting up the board, and a GUI written in tkinter(requires tkinter and tkinter.tix).

```
#!/usr/bin/python
# Sudoku board code, and Sudoku board generation code.
# TODO:
# - Sudoku Solver
# - GUI Load/Save game (DONE)
# - GUI Board Drawing (DONE)
# - GUI Board Sync (DONE)
# - GUI Board Interaction (DONE)
# - GUI End Game Mode
# - Reimplment SudokuBoard more efficently and with less complexity
import random
import time
import os
import tkinter.tix
import pickle
from tkinter import *
from tkinter.constants import *
from tkinter.tix import FileSelectBox, Tk

random.seed(time.time())

# There are probably a few bugs in this class, and it could be implemented
# better I think.
class SudokuBoard:
"""
Data structure representing the board of a Sudoku game.
"""

def __init__(self):
self.clear()

def clear(self):
"""
Empty the board.
"""
self.grid = [[0 for x in range(9)] for y in range(9)]
self.locked = []

def get_row(self, row):
return self.grid[row]

def get_cols(self, col):
return [y[col] for y in self.grid]

def get_nearest_region(self, col, row):
"""
Regions are 3x3 sections of the grid.
"""
def make_index(v):
if v rsize:
x = int(event.x/rsize)
if event.y > rsize:
y = int(event.y/rsize)
print(x,y)
if self.current:
(tx, ty) = self.current
#self.canvas.itemconfig(self.handles[ty][tx][0], fill=rgb(128,128,128))
self.current = (x,y)

# BUG: Changing the color of the background of a tile erases parts of
# the thick gridlines
#self.canvas.itemconfig(self.handles[y][x][0], fill=rgb(255,255,255))

def canvas_key(self, event):
print("Clack! (%s)" % (event.char))
if event.char.isdigit() and int(event.char) > 0 an

In general I think the code is clear, self-explanatory and well implemented. For some points that could use improvement:

-
SudokuBoard.get_nearest_region

• make_index is unnecessary, use v // 3 * 3 instead (in case you don't know, // means "integer division" in Python); (Edit: as @NolenRoyalty pointed out, keeping it is better for code clarity, though it can be simplified using the expression above)

-
Since this function is only used in set, you might as well make its results more convenient for it (by flattening the result):

def make_index(v):
    """Index of the closest row/column (to the top/left)"""
    return v // 3 * 3
return [c for y in self.grid[make_index(row):make_index(row)+3] for c in y[make_index(col):make_index(col)+3]]

This way the calling code will be much simpler:

if v in self.get_row(row) + self.get_cols(col) + self.get_nearest_region(col,row):
    raise ValueError()

Note also that I replaced the 3 for loops for a single if, using list concat to get all the invalid values (maybe with some repetition) and in to check if your value is in that list. Set union would work too: set(list1) | set(list2) | set(list3) (I'm referring to the builtin type set, not your SudokuBoard.set function).

-
In SudokuBoard.__str__, you could use enumerate to get the index of each element together with the element, while iterating, and using remainder (modulus) to detect third rows:

strings = []
for index,y in enumerate(self.grid):
    strings.append("%d%d%d %d%d%d %d%d%d" % tuple(y))
    if index % 3 == 2:
        strings.append('')
return '\n'.join(strings)

-
In sudogen_1, instead of repeateadly generating random numbers, you could just shuffle the interval and pick from it (since you don't want repeats):

board.clear()
r_vals = range(1,9) # Generate all values from 1 to 9
random.shuffle(r_vals) # Shuffle those values, to they will appear in random order
for y in range(0, 9, 3):
    for x in range(0, 9, 3):
        if not r_vals:
            return
        i = r_vals.pop() # Gets (and removes) one value from list
        try:
            board.set(random.randint(x, x+2), random.randint(y, y+2), i, lock=True)
        except ValueError:
            print("Board rule violation, this shouldn't happen!")

-
In sync_board_and_canvas I'd personally not use that if-else, but move it to the text parameter only (that's a bit subjective though):

self.canvas.itemconfig(self.handles[y][x][1], 
                       text=str(g[y][x]) if g[y][x] else '')

That's it. I have no experience with tkinter, so I limited my feedback to general aspects of your code. Maybe someone with better knowledge of it can contribute with that other part.

StackExchange Code Review Q#10908, answer score: 3