There are some very good things about this code.
It makes good use of library functionality, which is one of the big strengths of Python. A lot of programmers spend unnecessary amounts of time rewriting things that others have already done, so it's good to see the habit of just using things like
askopenfilename as provided.
I also like the use of comments. There are two traps that people fall into with comments: one is not writing any, and the other is writing comments that just say what the code is already saying. A comment like "Update the window" before
win.update() would be pointless, whereas your comment "See progress for each row" explains clearly and concisely what the intention behind that update is.
That said, I am slightly confused by two of your comments. It's almost midnight here and I may be misreading the code, but it looks to me as if the first
for loop is going from left to right one column at a time, and the second for loop is going one row at a time. If so, those comments are the wrong way round. It's important to make sure that comments are kept up to date and accurate, or they can mislead the person reading the code (even if it's future you.)
You mention it running quite slowly. Now the general rule is that when something runs slowly, you use a tool called a profiler to work out which bits of the code are taking time. I haven't run one on this code, but in my experience that
print line is probably taking a surprising amount of time. We always assume that text is easy for computers, which it kind of is, but if you think about it the computer has to set maybe a couple of thousand pixels to update that text for every one pixel that it's doing the greyscale calculation for. Although it's really useful to have such lines while developing something, it's usually worth taking them out once they have served their purpose.
Finally, I'll rattle through a few python best practices that are generally worth bearing in mind.
- Your function
displayimage does two jobs. It first loads an image from a file, and then creates a window and puts it on the screen. Now, you have made good use of python's multi-return functionality to let you do that cleanly, but usually best practice is that every function should do one thing and do it well. I would suggest cutting that function in half: one function could take a file name and return an image, and a second function should take an image and set up a window for it.
- Python programmers usually prefer naming things with snake_case for variable names and function names, which means join lowercase words with underscores. That is purely a convention, but sticking to conventions helps to read code more fluently.
- It is generally discouraged to use
import *. That is because it is not clear what comes from where. If I want to look at the documentation for, say,
Image then I don't know whether to look at the
graphics documentation or somewhere else. Even worse, there's a whole bunch of stuff in packages that you're not using, and it can trip you up if you don't realise it's there. For example you could imagine that your
graphics module could also have a function called
asksaveasfilename. (Ok, probably not that one, but it serves for an example) Then when you call
asksaveasfilename you get all sorts of confusion because it could call the wrong one. Even worse, if you update your packages then
graphics which didn't have
asksaveasfilename before might have it now, and the behaviour of your program changes in entirely unexpected ways.
I hope this is helpful.