Discussion:
Best use of "open" context manager
(too old to reply)
Rob Cliffe
2024-07-06 10:49:06 UTC
Permalink
Consider this scenario (which I ran into in real life):
    I want to open a text file and do a lot of processing on the lines
of that file.
    If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
I might write it like this:

try:
    with open(FileName) as f:
        for ln in f:
            print("I do a lot of processing here")
            # Many lines of code here .....
except FileNotFoundError:
    print(f"File {FileName} not found")
    sys.exit()

but this violates the principle that a "try" suite should be kept small,
so that only targeted exceptions are trapped,
not to mention that having "try" and "except" far apart decreases
readability.

Or I might write it like this:

try:
    f = open(FileName) as f:
    FileLines = f.readlines()
except FileNotFoundError:
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
for ln in File Lines:
        print("I do a lot of processing here")
        # Many lines of code here .....

but this loses the benefits of using "open" as a context manager,
and would also be unacceptable if the file was too large to read into
memory.

Really I would like to write something like

try:
    with open(FileName) as f:
except FileNotFoundError:
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
        for ln in f:
            print("I do a lot of processing here")
            # Many lines of code here .....

but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get
ValueError: I/O operation on closed file

I could modify the last attempt to open the file twice, which would
work, but seems like a kludge (subject to race condition, inefficient).

Is there a better / more Pythonic solution?

Best wishes
Rob Cliffe
2***@potatochowder.com
2024-07-06 11:09:45 UTC
Permalink
On 2024-07-06 at 11:49:06 +0100,
Post by Rob Cliffe
Is there a better / more Pythonic solution?
https://docs.python.org/3/library/fileinput.html

At least this attempts to abstract the problem of iterating over a file
(or multiple files) into a library routine. I've used it a little, but
I don't know the full depths of your use case and/or requirements.

HTH,
Dan
Alan Gauld
2024-07-06 11:43:40 UTC
Permalink
Post by Rob Cliffe
    If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
            print("I do a lot of processing here")
            # Many lines of code here .....
    print(f"File {FileName} not found")
    sys.exit()
but this violates the principle that a "try" suite should be kept small,
The try is small, it only has a single statement inside.
The compound block inside that statement should have its
own try/ecxepts but the outer one really only applies to
the with statement.

I certainly prefer this option to any of the others presented.
Post by Rob Cliffe
not to mention that having "try" and "except" far apart decreases
readability.
This is a valid concern although that's part of the reason
we use indentation. Compared to early BASIC and FORTRAN with massive
GOSUB type leaps (and often no indentation) it's very readable!

But its still preferable to having the multi-level indents below
or having to remember to close the file when finished (and
ensure that all possible paths do so.
Post by Rob Cliffe
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
Exactly! That's why using with is safer even if the
except is detached from the try.

You are also reading the entire file into memory which could
be an issue. And you are not catching any errors in the
read operations because the except only covers a missing
file.
Post by Rob Cliffe
Really I would like to write something like
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
            print("I do a lot of processing here")
            # Many lines of code here .....
I find that much less readable because the file handling
block is a long way from the open file line and has extra
control statements to mentally negotiate.

The advantage of the original version is that you can
ignore errors and read the code easily. You only need
to find the except clause if you need to know how errors
will be dealt with. But if comprehending the core
functionality of the code you can just ignore all
the error handling blocks.
Post by Rob Cliffe
Is there a better / more Pythonic solution?
All IMHO of course, but I think the current implementation
is the best of the options presented.
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos
Stefan Ram
2024-07-06 11:46:33 UTC
Permalink
Post by Rob Cliffe
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
            print("I do a lot of processing here")
            # Many lines of code here .....
but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get
ValueError: I/O operation on closed file
try:
f = open( FileName )
except FileNotFoundError:
print( f"File {FileName} not found" )
sys.exit()
else:
with f:
# put this into a separate function if it gets too long here.
for ln in f:
print( "I do a lot of processing here" )
            # Many lines of code here .....
Lawrence D'Oliveiro
2024-07-07 03:49:44 UTC
Permalink
Post by Stefan Ram
Post by Rob Cliffe
but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get ValueError: I/O operation on
closed file
f = open( FileName )
print( f"File {FileName} not found" )
sys.exit()
# put this into a separate function if it gets too long here.
print( "I do a lot of processing here" )
            # Many lines of code here .....
f = open(filename, "rt")
for ln in f :
... do your processing ...

1) Let the error exception be reported directly, whether it’s “file not
found”, or “permission error”, or some other reason; why bother to handle
it when you don’t even know what to do anyway?
2) Notice that a file open for reading automatically gets closed when it
goes out of scope (feature of CPython and all the other good
implementations).
Oscar Benjamin
2024-07-06 11:57:37 UTC
Permalink
On Sat, 6 Jul 2024 at 11:55, Rob Cliffe via Python-list
Post by Rob Cliffe
I want to open a text file and do a lot of processing on the lines
of that file.
If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
print("I do a lot of processing here")
# Many lines of code here .....
print(f"File {FileName} not found")
sys.exit()
but this violates the principle that a "try" suite should be kept small,
so that only targeted exceptions are trapped,
not to mention that having "try" and "except" far apart decreases
readability.
This is catching a targeted exception (FileNotFoundError) so I think
it is fine. If the intention is just to call sys.exit() on error then
I wouldn't worry too much about having too much code in the try. Just
make sure that you do this in any other place where you open a file as
well.

One possible improvement is that you could catch the exception and use
its filename attribute:

except FileNotFoundError as e
print(f"File {e.filename} not found")

That way if you did catch the wrong FileNotFoundError then at least
you print the correct filename.

Alternatively:

except FileNotFoundError as e
if e.filename != FileName:
raise # re-raise if not the intended exception
print(f"File {e.filename} not found")

For readability I would just move the many lines of code into a
separate function.

The reason to avoid having too much code in the try mainly applies to
situations where you are going to do something other than call
sys.exit() and the exception is overly generic like ValueError or
TypeError. If the exception can easily be raised by a bug or something
other than the intended cause then it is bad to catch exceptions
around a larger block of code.

If it is expected that the caller of a function might have good reason
to catch the exception and handle it somehow then it is better to make
a dedicated exception class and raise that instead. When there is only
one place in the code that raises a particular exception type and only
one place that catches it then it is usually going to be clear that
you are catching the expected exception.

--
Oscar
Thomas Passin
2024-07-06 13:40:49 UTC
Permalink
Post by Rob Cliffe
    I want to open a text file and do a lot of processing on the lines
of that file.
    If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
            print("I do a lot of processing here")
            # Many lines of code here .....
    print(f"File {FileName} not found")
    sys.exit()
but this violates the principle that a "try" suite should be kept small,
so that only targeted exceptions are trapped,
not to mention that having "try" and "except" far apart decreases
readability.
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
        print("I do a lot of processing here")
        # Many lines of code here .....
but this loses the benefits of using "open" as a context manager,
and would also be unacceptable if the file was too large to read into
memory.
Really I would like to write something like
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
            print("I do a lot of processing here")
            # Many lines of code here .....
but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get
ValueError: I/O operation on closed file
I could modify the last attempt to open the file twice, which would
work, but seems like a kludge (subject to race condition, inefficient).
Is there a better / more Pythonic solution?
I usually read the file into a sequence of lines and then leave the
open() as soon as possible. Something like this:

FILENAME = 'this_is_an_example.txt'
lines = None
if os.path.exists(FILENAME):
with open(FILENAME) as f:
lines = f.readlines()
# do something with lines

Of course, if you want to read a huge number of lines you will need to
be more thoughtful about it. Or make all the processing within the
open() block be a function. Then you just have one more line in the block.
Richard Damon
2024-07-06 15:01:11 UTC
Permalink
My thoughts is that if the "many lines of code" puts the except to far
from the try, then perhaps it would have made sense to factor out some
part there into a function.

Perhaps like:

try:
   with open(FileName) as f:
      for ln in f{
         process(ln)
except FileNotFoundError:
   print(f"File {FileName} not found:")
   sys.exit()

Now the "process" function has been factored out and can be well
documented as to what it is doing on each line, and this code can be
documented as running process on each line of the file.
Post by Rob Cliffe
    I want to open a text file and do a lot of processing on the lines
of that file.
    If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
            print("I do a lot of processing here")
            # Many lines of code here .....
    print(f"File {FileName} not found")
    sys.exit()
but this violates the principle that a "try" suite should be kept
small, so that only targeted exceptions are trapped,
not to mention that having "try" and "except" far apart decreases
readability.
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
        print("I do a lot of processing here")
        # Many lines of code here .....
but this loses the benefits of using "open" as a context manager,
and would also be unacceptable if the file was too large to read into
memory.
Really I would like to write something like
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
            print("I do a lot of processing here")
            # Many lines of code here .....
but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get
ValueError: I/O operation on closed file
I could modify the last attempt to open the file twice, which would
work, but seems like a kludge (subject to race condition, inefficient).
Is there a better / more Pythonic solution?
Best wishes
Rob Cliffe
--
Richard Damon
dn
2024-07-06 21:05:36 UTC
Permalink
Post by Rob Cliffe
    I want to open a text file and do a lot of processing on the lines
of that file.
    If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
            print("I do a lot of processing here")
            # Many lines of code here .....
    print(f"File {FileName} not found")
    sys.exit()
but this violates the principle that a "try" suite should be kept small,
so that only targeted exceptions are trapped,
Yes!
Post by Rob Cliffe
not to mention that having "try" and "except" far apart decreases
readability.
Uh-oh!

- and there's a bit of a hang-over for old-timers who had to take care
of file-locking within the application - we try to minimise 'time'
between opening a file and closing it (etc)!

As it seems the file is opened to read. Less relevant in this case, but
habits and styles of coding matter...
Post by Rob Cliffe
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
        print("I do a lot of processing here")
        # Many lines of code here .....
but this loses the benefits of using "open" as a context manager,
and would also be unacceptable if the file was too large to read into
memory.
So, now there are two concerns:
1 FileNotFoundError, and
2 gradual processing to avoid memory-full

- added to remembering to close the file.
Post by Rob Cliffe
Really I would like to write something like
    print(f"File {FileName} not found")
    sys.exit()
else: # or "finally:"
            print("I do a lot of processing here")
            # Many lines of code here .....
but this of course does not work because by the time we get to "for ln
in f:" the file has been closed so we get
ValueError: I/O operation on closed file
I could modify the last attempt to open the file twice, which would
work, but seems like a kludge (subject to race condition, inefficient).
Is there a better / more Pythonic solution?
Idea 1: invert the exception handling and the context-manager by writing
a custom context-manager class which handles FileNotFoundError
internally. Thus, calling-code becomes:

with...
for...
processing

Idea 2: incorporate idea of encapsulating "processing" into a
(well-named) function to shorten the number of lines-of-code inside the
with-suite.

Idea 3: consider using a generator to 'produce' lines of data
one-at-a-time. Remember that whilst context-managers and generators are
distinct concepts within Python, they are quite similar in many ways.
So, a custom generator could work like a context-manager or 'wrap' a
context-manager per Idea 1.

Building a custom-class (Idea 1 or Idea 3) enables the components to be
kept together, per the ideal. It keeps the try-except components close
and easy to relate. It is Pythonic (in the OOP style).
--
Regards,
=dn
Cameron Simpson
2024-07-07 01:08:33 UTC
Permalink
Post by Rob Cliffe
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
        print("I do a lot of processing here")
        # Many lines of code here .....
What about this:

try:
    f = open(FileName) as f:
except FileNotFoundError:
    print(f"File {FileName} not found")
    sys.exit()
with f:
... process the lines here ...

Remember, the `open()` call returns a file object _which can be used as
a context manager_. It is separate from the `with` itself.
Rob Cliffe
2024-07-07 21:22:49 UTC
Permalink
Post by Rob Cliffe
    FileLines = f.readlines()
    print(f"File {FileName} not found")
    sys.exit()
# I forgot to put "f.close()" here -:)
        print("I do a lot of processing here")
        # Many lines of code here .....
        print(f"File {FileName} not found")
        sys.exit()
        ... process the lines here ...
Remember, the `open()` call returns a file object _which can be used
as a context manager_. It is separate from the `with` itself.
Did you test this?
    f = open(FileName) as f:
is not legal syntax.
If you omit the "as f:"
it's legal, but doesn't work (trying to access the file after "with f"
raises the same
    ValueError: I/O operation on closed file.
I'm using Python 3.11.5.

Best wishes
Rob Cliffe
Cameron Simpson
2024-07-07 23:02:38 UTC
Permalink
Post by Rob Cliffe
Post by Cameron Simpson
Remember, the `open()` call returns a file object _which can be used
as a context manager_. It is separate from the `with` itself.
Did you test this?
is not legal syntax.
No. You're right, remove the "as f:".
Post by Rob Cliffe
it's legal, but doesn't work (trying to access the file after "with f"
raises the same
    ValueError: I/O operation on closed file.
This astounds me. Code snippet to demo this?

Here's a test script which I've just run now:

FileName = 'foo.txt'
try:
f = open(FileName)
except FileNotFoundError:
print(f"File {FileName} not found")
sys.exit()
with f:
for line in f:
print("line:", line.rstrip())

Here's the foo.txt file:

here are
some lines of text

Here's the run:

% python3 p.py
line: here are
line: some lines of text
Cameron Simpson
2024-07-08 02:45:21 UTC
Permalink
Post by Rob Cliffe
it's legal, but doesn't work (trying to access the file after "with f"
raises the same
    ValueError: I/O operation on closed file.
Just to this: of course. The with closes the file. But my version runs
the with after the try/except.
Rob Cliffe
2024-07-06 13:27:52 UTC
Permalink
Post by Oscar Benjamin
On Sat, 6 Jul 2024 at 11:55, Rob Cliffe via Python-list
Post by Rob Cliffe
I want to open a text file and do a lot of processing on the lines
of that file.
If the file does not exist I want to take appropriate action, e.g.
print an error message and abort the program.
print("I do a lot of processing here")
# Many lines of code here .....
print(f"File {FileName} not found")
sys.exit()
but this violates the principle that a "try" suite should be kept small,
so that only targeted exceptions are trapped,
not to mention that having "try" and "except" far apart decreases
readability.
This is catching a targeted exception (FileNotFoundError) so I think
it is fine. If the intention is just to call sys.exit() on error then
I wouldn't worry too much about having too much code in the try. Just
make sure that you do this in any other place where you open a file as
well.
One possible improvement is that you could catch the exception and use
except FileNotFoundError as e
print(f"File {e.filename} not found")
That way if you did catch the wrong FileNotFoundError then at least
you print the correct filename.
Good point, Oscar - thank you.  (Even if you did omit the colon on the
"except" line🙂.  I've often thought we should have "Python without
colons" as this is a mistake I frequently make.)
Post by Oscar Benjamin
except FileNotFoundError as e
raise # re-raise if not the intended exception
print(f"File {e.filename} not found")
Indeed, that covers all basis.
Post by Oscar Benjamin
For readability I would just move the many lines of code into a
separate function.
That may not always be convenient (e.g. if the many-lines-of-code needs
to access a lot of local variables) but fair enough.
Thanks for your answer.
Rob Cliffe
Post by Oscar Benjamin
The reason to avoid having too much code in the try mainly applies to
situations where you are going to do something other than call
sys.exit() and the exception is overly generic like ValueError or
TypeError. If the exception can easily be raised by a bug or something
other than the intended cause then it is bad to catch exceptions
around a larger block of code.
If it is expected that the caller of a function might have good reason
to catch the exception and handle it somehow then it is better to make
a dedicated exception class and raise that instead. When there is only
one place in the code that raises a particular exception type and only
one place that catches it then it is usually going to be clear that
you are catching the expected exception.
--
Oscar
Albert-Jan Roskam
2024-07-12 09:30:14 UTC
Permalink
Or like below, although pylint complains about this: "consider using
with". Less indentation this way.
f = None
try:
f = open(FILENAME)
records = f.readlines()
except Exception:
sys.exit(1)
finally:
if f is not None:
f.close()
Stefan Ram
2024-07-12 11:55:13 UTC
Permalink
Post by Albert-Jan Roskam
Or like below, although pylint complains about this: "consider using
with". Less indentation this way.
f = None
f = open(FILENAME)
records = f.readlines()
This try clause would also catch exception raised by the readlines
call, and I thought that this was what the OP was looking to avoid.
Post by Albert-Jan Roskam
sys.exit(1)
When this code is part of a library, it might not be appropriate
to make the whole application exit at this point.

Loading...