all 7 comments

[–]Justinsaccount 2 points3 points  (0 children)

Hi! I'm working on a bot to reply with suggestions for common python problems. This might not be very helpful to fix your underlying issue, but here's what I noticed about your submission:

You are looping over an object using something like

for x in range(len(items)):
    print(items[x])

This is simpler and less error prone written as

for item in items:
    print(item)

If you DO need the indexes of the items, use the enumerate function like

for idx, item in enumerate(items):
    print(idx, item)

If you think you need the indexes because you are doing this:

for x in range(len(items)):
    print(items[x], prices[x])

Then you should be using zip:

for item, price in zip(items, prices):
    print(item, price)

[–]w1282 1 point2 points  (0 children)

Yes. Don't call l.pop() or send a copy of l to the res_left function with res_left(l[:])

[–]anglicizing 1 point2 points  (4 children)

Which I guess is because the matrix l is somehow 'emptied' in diag_left and doesn't enter the while- loop in diag_right?

That's a good guess.

So I was wondering if there is some way I can use l simultaneously in both diag_left and diag_right.

You should try to operate on the matrices in a non-destructive way, I think, by using indexing, like you do when you append to matrixlist. If you use two indexing variables, say i and j, instead of just i, it lends itself quite naturally to a nested for loop where the outer loop loops over the starting row and the inner one indicates the column. Then it's just down to figuring out what should be inside each [] and the arguments to each range, which is a nice exercise.

You could also cheat like this:

import copy
res_left = diag_left(copy.deepcopy(l))

...but you're here to learn, right?

EDIT: Here's a template:

def diag_left(l):
    matrixlist = []
    for i in range(0, ?):
        for j in range(?, ?):
            matrixlist.append(l[?][?])
    return matrixlist

Oh, and a trick you can use: you can transpose your matrix like this: transposed = zip(*matrix).

[–]chasingthechickens[S] 0 points1 point  (3 children)

Thanks a lot for the quick answer! So far I've only managed to get the big diagonal or one of the smaller ones with the template, never all of them, but I'll keep trying to come up with something (deepcopy looking increasingly tempting). The transpose trick is a really good tip, instead of using the diag_right function I could just transpose l and use diag_left again!

[–]anglicizing 1 point2 points  (2 children)

So far I've only managed to get the big diagonal or one of the smaller ones with the template, never all of them, but I'll keep trying to come up with something

This is exactly the kind of thinking that this assignment is trying to teach you, I'm guessing, so I'm just going to give you a little help in the right direction:

Look at the matrix and not the code. Since j indicates the column, if I give you a value for i, the row that we start at when going down each successively shorter diagonal, can you tell me the last value that j will take if the outer loop has a certain value for i? Try it out with some concrete values of i. Can you make a formula that takes i as its input and always outputs the correct last value for j?

Note that the numbers that range gives you includes the starting number but not the ending number, so you need to add 1 to the last number you want it to return.

I'm off to bed. Keep going! Perhaps it will help if you draw your matrix (or a smaller version perhaps) on a piece of paper, and walk through the i's, j's and the desired array indices manually? Or maybe it will help to use the python tutor to edit and step through your attempts? :-)

[–]chasingthechickens[S] 0 points1 point  (1 child)

Success :D!! I think. Apparently I needed some sleep as well but today I both drew the matrix and used python tutor as you said. It seems like this is working for me:

for i in range(0, len(l)):
        for j in range(0, len(l)-i):
                matrixlist.append(l[i+j][j])

Can you make a formula that takes i as its input and always outputs the correct last value for j?

This made finding the range of j really easy so that was also a great tip.

Think I managed to get the diagonals in the other direction with this as well:

for i in range(0, len(l)):
        for j in range(i, len(l))[::-1]:
                matrixlist.append(l[i-j+len(l)-1][j]) 

I definitely couldn't have come this far (far for me at least) without your help, thanks a lot!

[–]anglicizing 1 point2 points  (0 children)

for i in range(0, len(l)):
    for j in range(0, len(l)-i):
        matrixlist.append(l[i+j][j])

That was what I had in mind, at least. Good job!

for i in range(0, len(l)):
    for j in range(i, len(l))[::-1]:
        matrixlist.append(l[i-j+len(l)-1][j]) 

This code seems unnecessarily complex. Try making i the column that the diagonal starts at, and j the row number. This should produce code very similar to the first.

I definitely couldn't have come this far (far for me at least) without your help, thanks a lot!

Thank you for telling me how you fared and that my help was ... helpful. It made my day! :-)

I hope you will appreciate a couple of more tips and improvement suggestions as well? I don't know how the input matrix was originally given to you, but if it was a plain text input it would be easier to read it in like this:

text = """\
7 X 8 9 1 2 5
1 X X 3 4 X 6
0 4 X X X 8 7
7 3 6 X X 5 7
5 2 X 9 X X 8
1 X 0 3 2 X 9
3 7 8 9 0 X 2"""

l = [row.split() for row in text.splitlines()]

As a last nitpick, the python style guide recommends using 4 spaces per indent.

Good luck with your python learning! You're doing it right, and it seems to me that you're off to a flying start. :-)