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)

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):

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:

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

                col_start = floor(x / 3) * 3
                row_start = (x % 3) * 3

                if sum(pos[col_start:col_start + 3,row_start:row_start + 3,y]) == 1:
                    index = nonzero(pos[col_start:col_start + 3,row_start:row_start + 3,y]);
                    set(col_start + index[0][0],row_start + index[1][0],y)

def only_pos_for_it():
    # This point can only be x
    for x in range(9):
        for y in range(9):
            if sum(pos[x,y,:]) == 1:

# main method
if __name__ == "__main__":
    # Init some variables
    feld = zeros((9,9),dtype=uint8) # field with the values were sure about
    pos = ones((9,9,9),dtype=bool_) # values which are still possible
    old_pos = 0


    # Let it run
    while any(old_pos != pos):
        old_pos = copy(pos)


Some comments on it:

  • I know that from NumPy import * is bad practise but its just so much faster to write.
  • Am I using the lambdas in a correct manner? I want it to be flexible to add more advanced solving algorithms later on.
  • Please tell me just anything you would change to make better use of the language (speed and readability wise).

I know that in terms of data structures this could be optimized to use something like this

class point:
    def __init__(self)
        self.val = uint8
        self.pos = zeros(9,dtype=bool_)

but then I would lose all the nice indexing that NumPy offers.

  • 1
    \$\begingroup\$ Why don't you check out Peter Norvig's famous Sudoku Solver written in Python for inspiration? The implementation is very Pythonic and is interesting code. He also includes a bunch of test cases that can be of interest to you in verifying that your code is working. \$\endgroup\$ Jul 9, 2014 at 18:17
  • \$\begingroup\$ @Pierre-AntoineLaFayette. Very nice link. Thank you I'll look into it. \$\endgroup\$
    – magu_
    Jul 10, 2014 at 6:02

1 Answer 1


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()]
  • \$\begingroup\$ Thank you very much for your comments. I tried to follow them as good as possible. The thing with global var I def. should have known. The filenames were an artifact from the beginning. You're also totally right about the whole transpose story. I tried to change the read into a for loop style, is that what you meant by it? Is the write fine or should I change it? \$\endgroup\$
    – magu_
    Jul 9, 2014 at 14:29
  • \$\begingroup\$ I've rolled back Rev 4 → 3. What you can and cannot do after receiving answers \$\endgroup\$ Jul 9, 2014 at 14:30
  • \$\begingroup\$ Ah ok. Does this mean I should open a new question with the code and simply link it to this one? \$\endgroup\$
    – magu_
    Jul 9, 2014 at 14:33
  • 1
    \$\begingroup\$ read_and_construct() is still bad, though at least it is honestly named. However, opening the file repeatedly inside zip() is bizarre. \$\endgroup\$ Jul 9, 2014 at 14:33
  • \$\begingroup\$ Yes, write a new question and cross-link them. However, I'd suggest waiting a while first to give others a chance to answer, and yourself a chance to think more clearly first. \$\endgroup\$ Jul 9, 2014 at 14:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.