1

I have a matrix or list of lists:

x = [[1,2,3], 
     [2,3,1], 
     [3,1,2]]

My goal is to check whether

1) Each column of the matrix contains, each of the whole numbers from 1 to n exactly once.

2) Each row of the matrix contains each of the whole numbers from 1 to n exactly once.

This is the problem exercise that I encountered when I was solving Udacity's intro to programming course. This is my solution to the problem. I know that this is long and inefficient. So what is the short and efficient way to do this problem??

def check(p):
    j = 0
    for e in p:
        i = 1 + j
    s = str(p)

    if s.find('.')!= -1:
        return False

    while i < len(p):
        if p[i] == e:
            return False
        if p[i] > len(p) or p[i] < 1:
            return False
        i += 1
        j += 1
    return True

def check_sudoku(p):
    i = 0
    z = []
    a = []
    x = 0
    for e in p:
        r = check(e)
        if r == False:
            return r

    #Below here is to transpose the list
    while x < len(p):
        z.append(1)
        x += 1
    while i < len(p):
        for e in p:
            a.append(e.pop())
        z[i] = a
        i +=  1
        a = []

    #Below here is to check the transpose
    for g in z:
        r = check(g)
        if r == False:
            return r
    return True
PM 2Ring
  • 54,345
  • 6
  • 82
  • 182
dangerous
  • 171
  • 1
  • 1
  • 11
  • 1
    1) Your indentation is wrong. 2) Use `#` in Python to start a comment, not `\\`. 3)There's a better way to check the list contents that doesn't require converting to string. 4)You can use zip(*list2D) to transpose a list of lists. – PM 2Ring Feb 09 '15 at 07:11
  • @PM2Ring - Hi Sure I will look into these errors. It would be great if you could write an answer with efficient solution. I am pretty new to programming and python – dangerous Feb 09 '15 at 07:15
  • 2
    @user00000341 the answer in the link you shared is irrelevant. – Ozgur Vatansever Feb 09 '15 at 07:16
  • @PM2Ring - I have edited – dangerous Feb 09 '15 at 07:27
  • 3
    Hopefully, I haven't destroyed your code's logic with my editing. :) BTW, it's better to do `if not r:` than `if r == False:`; similarly `if r:` is better than `if r == True:`. – PM 2Ring Feb 09 '15 at 07:41
  • @PM2Ring - now I got it!! – dangerous Feb 10 '15 at 01:47

3 Answers3

4

You can simplify your check program like this:

def check_valid(matrix):
    n = len(matrix[0])
    valid_num = range(1, n+1)     # make a list of valid number from 1 to n

    # sort rows and column of matrix in ascending order and compare with valid_num 
    for line in matrix:
        if sorted(line) != valid_num:
            return False
    for column in zip(*matrix):
        if sorted(column) != valid_num:
            return False

    # If all rows and column is valid, then 
    return True 

EDIT: follow @vocalno KISS rule but faster and work for both numbers and characters

def check_valid2(matrix):
    n = len(matrix)

    for line in matrix+zip(*matrix):
        if len(set(line)) != n:
            return False

    return True

In[9]: %timeit for m in (x, y): check_valid_mine(m)
100000 loops, best of 3: 8.55 µs per loop

In[10]: %timeit for m in [x,y]: check_valid2(m)
100000 loops, best of 3: 5.77 µs per loop

EDIT 2: all about the speed

def check_valid3(matrix):
    n = len(matrix)

    for line in matrix+zip(*matrix):
        if not len(set(line)) - n:
            return False

    return True

In[19]: %timeit for m in [x,y]: check_valid3(m)
100000 loops, best of 3: 2.29 µs per loop
dragon2fly
  • 2,309
  • 19
  • 23
  • Will this solution work for characters as well. for eg: [[a,b,c],[b,c,a]] – dangerous Feb 09 '15 at 07:28
  • @jameadreen You will have to change `valid_num` to `valid_char=[chr(c) for c in range(ord('a'), ord('c')+1)]` – dragon2fly Feb 09 '15 at 07:32
  • I have 2 queries. what if why do you choose len(matrix[0]) instead of len(matrix). the character solution you have given in the comments doesnt work for characters that are greater than 'c', how should I change my code, so that even other characters are accepted. – dangerous Feb 10 '15 at 01:13
  • @dangerous `len(matrix[0]) == len(matrix) == n` so it doesn't matter what you use. I've tried with n = 5 and characters from 'a' to 'e' and it worked fine. What exactly is your problem? (Traceback, error message)? – dragon2fly Feb 10 '15 at 01:27
  • Consider this case, z = [[8,9,10], [9,10,8], [10,8,9]]. For this case your solution doesnt fit. and more over I have to write a code that fits both the alphabets and numbers. – dangerous Feb 10 '15 at 01:38
  • @dangerous in that case, your numbers are not in range 1 -> n and out of your question. `valid_num = range(8, 10+1)` will work for that case. In order to work with both number and character within a specific range, you have to define that range and make it as a parameter of function – dragon2fly Feb 10 '15 at 01:49
  • I have posted an answer, please correct me if I am wrong. – dangerous Feb 10 '15 at 01:59
  • @dragon2fly - again you are not considering unique elements. – dangerous Feb 11 '15 at 04:03
  • @dangerous `unique` element is equivalent to `len(set(line)) = n`. If there are more than n elements exist in matrix, all check_valid function we wrote will False – dragon2fly Feb 11 '15 at 04:23
  • @volcano- now I got it, can you explain the query I have asked in comments. dragon fly- Yes I understood your solution now. – dangerous Feb 11 '15 at 05:38
  • @dragon2fly , dropping a set comparison is a nice touch – volcano Feb 11 '15 at 05:45
  • @dragon2fly , you are adding lists - which creates a new list that contains all the elements of the original lists. Again, it works - but it is not a good practice. – volcano Feb 11 '15 at 05:46
  • @volcano I know that you're telling me to use `chain`. It doesn't create a new list but it also slow down the code. Thanks for your suggestion – dragon2fly Feb 11 '15 at 09:19
  • @dragon2fly, you guessed wrong. I am telling to use nested loop - it does the trick too – volcano Feb 11 '15 at 09:29
  • @volcano Yeah, you're right. I thought of that but I was afraid of repeating myself. Thanks again. – dragon2fly Feb 11 '15 at 09:37
2

Here's my solution

I cycle over rows and columns, using zip to flips matrix and chain - to combine 2 lists. if either row or column contain values not in valid_set,

set(row) - valid_set

expression will yield non-empty set - turned to False by not- and all cycle will end

from itertools import chain
def check_valid(matrix, n):
   valid_set = set(range(1, n+1))
   return all(not(set(row) - valid_set) for row in chain(matrix, zip(*matrix)))

EDIT:
Sorry, misread the question; here's correct answer - all exits on first False

from itertools import chain
def check_valid(matrix):
   valid_set = set(range(1, len(matrix)+1))
   return all(set(row) == valid_set for row in chain(matrix, zip(*matrix)))

EDIT 2:

Out of curiosity - I timed sort vs set approach

In [74]: x = [[1,2,3], 
   ....:      [2,3,1], 
   ....:      [3,1,2]]

In [79]: %timeit for row in x: sorted(row)
1000000 loops, best of 3: 1.38 us per loop

In [80]: %timeit for row in x: set(row)
1000000 loops, best of 3: 836 ns per loop

set is about 30% faster

EDIT 3: I fixed dangerous solution and updated mine

In [132]: def check_valid(matrix):
    valid_num = np.unique(np.array(matrix)).tolist() # selects unique elements
    for line in matrix:
        if sorted(line) != valid_num:
            return False
    for column in zip(*matrix):
        if sorted(column) != valid_num:
            return False
    return True
In [136]: %timeit for m in (z, d): check_valid(m)
10000 loops, best of 3: 57.8 us per loop

In [115]: def check_valid_mine(matrix):                                                  
       valid_set = set(chain(*matrix))
       return all(set(row) == valid_set for row in chain(matrix, zip(*matrix)))

In [137]: %timeit for m in (z, d): check_valid_mine(m)
100000 loops, best of 3: 8.96 us per loop

Bottom line? KISS - keep it simple stupid

Explaining the matrix transposition by zip()

  • operator before iterable argument in a function call expands this argument list of positional arguments, essentially

    zip(*matrix)

means

zip(matrix[0], matrix[1], matrix[2], ...)

thus rearranging columns into rows Check out this answer for detailed explanation of * and ** operators

Community
  • 1
  • 1
volcano
  • 3,578
  • 21
  • 28
  • That will work too. But I would rather use _all_ with _not set(row) - valid_set_. – volcano Feb 09 '15 at 07:48
  • That's even better. :) – PM 2Ring Feb 09 '15 at 07:50
  • @PM2Ring, thanks. Though it's a little too high for OP, I admit – volcano Feb 09 '15 at 07:51
  • nice way with set and itertools :) – dragon2fly Feb 09 '15 at 07:52
  • True, the `itertools.chain()` might be a little too advanced, but it doesn't hurt to learn that these functions exist. But I reckon the set stuff is ok. Maybe you should add a solution that doesn't use itertools... – PM 2Ring Feb 09 '15 at 07:53
  • @PM2Ring that's why I posted my version - and added explanations – volcano Feb 09 '15 at 07:55
  • What is chain and all function doing here?? I am not able to understand these two functions. – dangerous Feb 09 '15 at 08:22
  • @jameadreen in short, `chain` return an iterater which makes both lists `matrix` and `zip(*matrix)` look like there is only one list. So instead of using two `for loop` to check, you just need one. In long, you need to learn about key word `yield`, `generator`, and have a look at https://docs.python.org/2/library/itertools.html#itertools.chain and https://docs.python.org/2/library/stdtypes.html#set – dragon2fly Feb 09 '15 at 08:29
  • _chain_ allows to iterate over collection of iterables - matrix and rotated matrix in this case, - as over sum of those iterables. _all_ function checks whether all elements of iterable may be evaluated to _True_; it stops on first _False_ condition. You should read it here https://docs.python.org/2.7/library/index.html – volcano Feb 09 '15 at 08:33
  • @dragon2fly and volcano - thanks for this detailed explanation. Now I got everything cleared. – dangerous Feb 09 '15 at 09:39
  • @PM2Ring and Volcano - I have posted an answer that, please correct me if Iam wrong. – dangerous Feb 10 '15 at 02:00
  • @dangerous and Volcano - I have posted an even simpler answer, no need to import anything, please check it out. BTW, you shouldn't care so must about the speed when you just start with python. `57.8 us` and `8.96 us` is all within a blink of eyes. And to tell the truth, `all()` is slow :D – dragon2fly Feb 11 '15 at 02:21
  • @dragon2fly, maybe without *all* it's faster - it is just more curt. But I'll buy it. As about the speed - you noticed microseconds, but you failed to notice that it's it's 6 times faster! My main objection was over-complication - but unless you learn to think of code in terms of efficiency early, you may acquire a lousy habit of writing cluttered and inefficient code. – volcano Feb 11 '15 at 05:42
  • Actually, _set(chain(*matrix))_ is an overkill, _set(matrix[0]))_ does the trick - ans shaves off some overhead – volcano Feb 11 '15 at 06:01
  • @volcano, sorry that you thought it curt but I think you misunderstood my point. I agree that it's 6 times slower. But even if it 50 time slower, it is still in the blink of eyes. Maintain `Readability Counts` is important although the most readable code is not always the most efficient code. But when speed and memory is become a problem, try to write the most efficient code like you is the best because `Although Practicality Beats Purity`. – dragon2fly Feb 11 '15 at 09:17
0

I was searching for a solution that perfectly matches both characters and numbers and I came with a solution using numpy library. I slightly modified dragon2fly solution. Please correct me if I am wrong. I checked the solution for both z and d values and it gave correct results.

 import numpy as np

z = [[8,9,10],  # example case with numbers
     [9,10,8], 
     [10,8,9]]


d =  [['a','b','c'], # example case with characters
      ['b','c','a'],
      ['c','a','b']]

def check_valid(matrix):
    valid_num = list(np.unique(matrix)) # selects unique elements


    # sort rows and column of matrix in ascend order and compare with valid_num 
    for line in matrix:
        if sorted(line) != valid_num:
            return False
    for column in zip(*matrix):
        if sorted(column) != valid_num:
            return False

    # If all rows and column is valid, then 
    return True
dangerous
  • 171
  • 1
  • 1
  • 11
  • Nice with `unique`. Now the character don't need to be in any specific range. – dragon2fly Feb 10 '15 at 02:04
  • OK, here's my evil speak: - This solution does not work - _list(np.unique(np.array(matrix)).tolist())_ does the trick - You are using high-end *numpy* where simple *set* will do - You are sorting lists - that is a relatively heavy and unnecessary (here) operation. On tiny *z[0]* - which is already sorted! - it took 1.5 time of *set*. On z[1] and z[2] it took additional 10% - You could have done it in 2 nested loops - instead of 2 separate loops - *for m in (matrix, zip(*matrix)):* – volcano Feb 10 '15 at 17:31
  • @volcano type(list(np.unique(z))) is a list and type(list(np.unique(np.array(z)).tolist()) ) is also a list. then what is the difference between the two?? – dangerous Feb 11 '15 at 00:29
  • @volcano - also can you explain me the concept behind zip(*matrix), how does it transposes a matrix?? and also could you also explain what is going on with chain(*matrix)?? I understand that * and ** passes arguments to the function. but in this context I am unable to get the concept. – dangerous Feb 11 '15 at 00:53
  • @dangerous, see my answer for explanation – volcano Feb 11 '15 at 05:55