32

I have the following code:

import glob, os
for file in glob.glob("\\*.txt"):
    if os.access(file, os.R_OK):
        # Do something
    else:
        if not os.access(file, os.R_OK):
            print(file, "is not readable")
        else:
            print("Something went wrong with file/dir", file)
        break

But I'm not entirely sure if this the right way to do it. Is it better to use try and catch the error? If so, how do I try for readability? Note the break in my else statement. As soon as a file can't be read I want to abort the loop.

3
  • 7
    You could try opening the file for reading and catching the resulting exception (if any). That would also be more robust than your current approach, since the file can become unreadable (or even disappear) between the two calls to os.access(). Commented Aug 18, 2015 at 13:17
  • 2
    Are you actually reading the file in the # Do something section or do you only want to test its readability? Commented Aug 18, 2015 at 13:37
  • @PM2Ring I'm actually reading the file there. Commented Aug 18, 2015 at 14:51

4 Answers 4

57

A more explicit way to check if file is actually a file and not directory for example, and it is readable:

from os import access, R_OK
from os.path import isfile

file = "/some/path/to/file"

assert isfile(file) and access(file, R_OK), \
       f"File {file} doesn't exist or isn't readable"
Sign up to request clarification or add additional context in comments.

3 Comments

I prefer this method even if it leaves room for the file to disappear or change before you open it.
Beware, the assert line is not executed if Python is executed in optimized mode by passing the -O option (docs.python.org/3/reference/simple_stmts.html#assert)
good point on assert, this was just a demonstration, you could change to an if statement.
20

For me, using a try-except at the same scope as one would use an if-else gains no readability. The value of exceptions is that they can be caught at a higher level in the call tree.

Moving out just one level, we avoid the break statement:

import glob, os
try:
    for file in glob.glob("\\*.txt"):
        with open(file) as fp:
            # do something with file
except IOError:
    print("could not read", file)

But the true genius of exceptions is when the code simply disappears:

# Operate on several files
# SUCCESS: Returns None
# FAIL: Raises exception
def do_some_files():
    for file in glob.glob("\\*.txt"):
        with open(file) as fp:
            # do something with file

Now it is the calling program's responsibility to display a useful error message on failure. We have removed responsibility for dealing with failure completely out of this code and into a whole other realm.

In fact, one can move the responsibility completely out of our program and into the interpreter. In that case, the interpreter will print some useful error message and terminate our program. If Python's default message is good enough for your users, I recommend not checking for errors at all. Thus, your original script becomes:

import glob, os
for file in glob.glob("\\*.txt"):
    # Do something

9 Comments

Your kung-fu is better, have to admit.
As an answer to your last suggestion, wouldn't the interpreter then skip unreadable files rather than halting the loop and stopping? Because we never define what we need to do when an error occurs
No, @BramVanroy, the final short script would not skip unreadable files. The presumed open() inside #Do Something would raise an exception when trying to open an unreadable file. This exception would be caught by the interpreter which would print an error message and exit. That is, not coincidentally, precisely what the program in your question does: print an error message and exit. The only difference between the behavior of my short version and your long version is the text of the error message.
What about an error like IOError: [Errno 2] No such file or directory: 'XXXAF1D4.TMP'? That's a temp file from one of 20 different functions that were called. How can you know what failed without context?
@JeffreyGoldberg - Not really, for several reasons. 1) If you use the with open() as fd syntax, the file will be closed upon exiting the with block, regardless of how it is exited. 2) if you store the file object as a local, the interpreter will close the file when the stack frames that reference it are destroyed by the exception handler, and 3) the OS will close the file when the process exits. The only case I can imagine catching the exception is if the IO failure during read requires some non-standard cleanup (say, it is a device file and .close() doesn't sufficiently reset it.)
|
11

In Python culture, it's more common to ask forgiveness, not permission, so it's preferable to catch the exception:

for filename in glob.glob('*.txt'):
    try:
        with open(filename) as fp:
            # work with the file

    except IOError as err:
        print "Error reading the file {0}: {1}".format(filename, err)
        break

That way you will also avoid any double-checks or race conditions.

2 Comments

The ask forgiveness, not permission link is broken.
@Gautam replaced with the same author's same talk from 2012.
2
try:
        # check to see if file is readable
        with open(filename) as tempFile:





except Exception as e:
        print e
        # here you can modify the error message to your liking

This is usually what I do. It is robust and straight forward

2 Comments

Oops that issue should be addressed now
Never ever use an "except Exception" without straight "raise" just after !

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.