Claudiu
Claudiu

Reputation: 229511

File open: Is this bad Python style?

To read contents of a file:

data = open(filename, "r").read()

The open file immediately stops being referenced anywhere, so the file object will eventually close... and it shouldn't affect other programs using it, since the file is only open for reading, not writing.

EDIT: This has actually bitten me in a project I wrote - it prompted me to ask this question. File objects are cleaned up only when you run out of memory, not when you run out of file handles. So if you do this too often, you could end up running out of file descriptors and causing your IO attempts at opening files to throw exceptions.

Upvotes: 11

Views: 4732

Answers (6)

Escualo
Escualo

Reputation: 42142

Even though it works as expected, I think it fails in two counts:

  1. Your code will not scale up seamlessly, because you are reading the entire file into memory, and this may or may not be necessarily what you want.
  2. According to the Zen of Python (try import this in the Python prompt to retrieve it) "explicit is better than implicit" and, by failing to explicitly close the file, you could confuse someone who, down the road, will be left with your code for maintenance.

It really helps being explicit! Python encourages explicit style.

Other than that, for a throwaway script, your style makes sense.

Maybe you will benefit from this answer.

Upvotes: 2

Vinay Sajip
Vinay Sajip

Reputation: 99465

No, it's perfectly reasonable Python style IMO, as per your reasoning.

Update: There are a lot of comments here about whether file objects get tidied up straight away or not. Rather than speculate, I did some digging. Here's what I see:


From a comment in Python's object.h:

The macros Py_INCREF(op) and Py_DECREF(op) are used to increment or decrement reference counts. Py_DECREF calls the object's deallocator function when the refcount falls to 0

Looking in Python's fileobject.c:

The function table for file objects points to function file_dealloc. This function calls close_the_file, which in turn closes the file.


So it seems reasonable to state that at the moment, on CPython, when there are no more references to a file object, it's closed without any delay. If you think this interpretation is wrong, please post a comment indicating why you feel that way.

Upvotes: 3

Imagist
Imagist

Reputation: 18524

The code works exactly as you say it does, but it's bad style nevertheless. Your code relies on assumptions which may be true now, but won't always be true. It's not impossible that your code will be run in a situation where the file being opened and not close does matter. Is it really worth that risk just to save 1 or 2 lines of code? I don't think so.

Upvotes: 4

0x89
0x89

Reputation: 2920

Just for the record: This is only slightly longer, and closes the file immediately:

from __future__ import with_statement

with open(filename, "r") as f:
    data = f.read()

Upvotes: 29

sepp2k
sepp2k

Reputation: 370407

It is true that it will close eventually, but eventually might not be soon enough. Especially if you're using this inside a loop, the system might run out of file handles before the GC gets to the file objects.

Upvotes: 7

Corey Goldberg
Corey Goldberg

Reputation: 60634

looks fine to me.. I read files like that often.

Upvotes: 1

Related Questions