HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Hints to make Sudoku solver more Pythonic

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pythonicmakemorehintssolversudoku

Problem

As you will see, I am not very familiar with Python and NumPy but want to learn it.

The following code is a very basic Sudoku solver which works fine for simple examples. Although it runs, I still have the feeling it is not very Pythonic.

```
from numpy import *
import sys

def set(col,row,wert):
# Set the value in field and adjust the possibility tensor
pos[col,row,:] = 0 # the field itself
pos[:,row,wert] = 0 # the col
pos[col,:,wert] = 0 # the row

# The 3x3 block
col_start = floor(col / 3) * 3
row_start = floor(row / 3) * 3
pos[col_start:col_start + 3,row_start:row_start + 3,wert] = 0

# Write down the value
feld[col,row] = wert + 1

def read(name):
# Read in the stuff and create field
fid = open('in','r')
for col in range(9):
line = fid.readline()

for row in range(9):
if line[row] is not '0':
set(row,col,int(line[row]) - 1)
fid.closed

def write(name):
# Write the output into a file
fid = open("out","w")

for col in range(0,9):
for row in range(0,9):
fid.write(str(feld[row,col]))
fid.write('\n')
fid.closed

def check():
# Check the solution for errors
print("%i Missing" % (81 - count_nonzero(feld)))
neq = lambda x: count_nonzero(unique(x)) != count_nonzero(x)

for x in range(9):
if neq(feld[:,x]):
print("Error in row %i" % (x))

if neq(feld[x,:]):
print("Error in col %i" % (x))

col_start = floor(x / 3) * 3
row_start = (x % 3) * 3
if neq(feld[col_start:col_start + 3,row_start:row_start + 3]):
print("Error in block %i / %i" % (col_start/3,row_start/3))

def only_pos_left():
# This point is the only one that can be x
for x in range(9):
for y in range(9):
if sum(pos[x,:,y]) == 1:
set(x,nonzero(pos[x,:,y])[0][0],y)

if sum(pos[:,x,y]) == 1:

Solution

I noticed a few suspicious problems with your coding style. There's plenty to talk about even without analyzing the sudoku solver portion of your program.

Your functions are actually more like procedures that act on global variables. For example, your read() doesn't return anything, and your write() doesn't take a parameter for the matrix to be written. Although set() takes several parameters, the object it mutates is not one of the parameters — it operates on the global pos instead. Similarly, check(), only_pos_left(), and only_pos_for_it() take no parameters at all, which is a bad sign.

Even when read() and write() take a parameter called name (you probably meant "filename"), they ignore the parameter altogether! The filenames 'in' and 'out' are hard-coded.

Your read() and write() each has a file descriptor leak. fid.closed does not close the file handle. You probably meant fid.close(). But really, you should almost always prefer to open files using a with block instead, so you would never need to worry about having to close them again.

Your read() and write() counterintuitively transpose rows ↔︎ columns. I suppose those transpositions cancel each other out, and therefore have no effect. However, it makes it hard for others to think about or discuss your code.

read() tries to do more than just reading the file. The function should do one thing only: return a 2-dimensional matrix. Converting characters to numbers could also be acceptable. However, constructing the data structure you want to use to represent the sudoku puzzle should be a separate function. The fact that read() also subtracts one from each value makes it more disconcerting. Perhaps you do want to have constraints in the 0 to 8 range, but such a transformation definitely doesn't fall under "reading".

To summarize some of the suggestions above…

def read(filename):
    """ Return the contents of the file as a 2-D array. """
    with open(filename) as f:
        return [map(int, row.rstrip()) for row in f.readlines()]

Code Snippets

def read(filename):
    """ Return the contents of the file as a 2-D array. """
    with open(filename) as f:
        return [map(int, row.rstrip()) for row in f.readlines()]

Context

StackExchange Code Review Q#56558, answer score: 7

Revisions (0)

No revisions yet.