48

Suppose I have a function that does things with a text file - for example reads from it and removes the word 'a'. I could either pass it a filename and handle the opening/closing in the function, or I could pass it the opened file and expect that whoever calls it would deal with closing it.

The first way seems like a better way to guarantee no files are left open, but prevents me from using things like StringIO objects

The second way could be a little dangerous - no way of knowing if the file will be closed or not, but I would be able to use file-like objects

def ver_1(filename):
    with open(filename, 'r') as f:
        return do_stuff(f)

def ver_2(open_file):
    return do_stuff(open_file)

print ver_1('my_file.txt')

with open('my_file.txt', 'r') as f:
    print ver_2(f)

Is one of these generally preferred? Is it generally expected that a function will behave in one of these two ways? Or should it just be well documented such that the programmer can use the function as appropriate?

37

Convenient interfaces are nice, and sometimes the way to go. However, most of the time good composability is more important than convenience, as a composable abstraction allows us to to implement other functionality (incl. convenience wrappers) on top of it.

The most general way for your function to use files is to take an open file handle as parameter, as this allows it to also use file handles that are not part of the filesystem (e.g. pipes, sockets, …):

def your_function(open_file):
    return do_stuff(open_file)

If spelling out with open(filename, 'r') as f: result = your_function(f) is too much to ask of your users, you could choose one of the following solutions:

  • your_function takes an open file or a file name as parameter. If it is a filename, the file is opened and closed, and exceptions propagated. There is a bit of an issue with ambiguity here which could be worked around using named arguments.
  • Offer a simple wrapper that takes care of opening the file, e.g.

    def your_function_filename(file):
        with open(file, 'r') as f:
            return your_function(f)
    

    I generally perceive such functions as API bloat, but if they provide commonly used functionality, the gained convenience is a sufficiently strong argument.

  • Wrap the with open functionality in another composable function:

    def with_file(filename, callback):
        with open(filename, 'r') as f:
            return callback(f)
    

    used as with_file(name, your_function) or in more complicated cases with_file(name, lambda f: some_function(1, 2, f, named=4))

  • 5
    The only drawback of this approach is that sometimes the name of the file-like object is needed, e.g. for error reporting: end users prefer to see "Error in foo.cfg(12)" rather than "Error in <stream@0x03fd2bb6>(12)". An optional "stream_name" argument to your_function can be used in this regard. – Tibo Nov 11 '14 at 8:48
21

The real question is one of completeness. Is your file processing function the complete processing of the file, or is it just one piece in a chain of processing steps? If it is complete in and of its own, then feel free to encapsulate all file access within a function.

def ver(filepath):
    with open(filepath, "r") as f:
        # do processing steps on f
        return result

This has the very nice property of finalizing the resource (closing the file) at the end of the with statement.

If however there is possibly a need for processing an already-open file, then the distinction of your ver_1 and ver_2 makes more sense. For example:

def _ver_file(f):
    # do processing steps on f
    return result

def ver(fileobj):
    if isinstance(fileobj, str):
        with open(fileobj, 'r') as f:
            return _ver_file(f)
    else:
        return _ver_file(fileobj)

This kind of explicit type testing is often frowned upon, especially in languages like Java, Julia, and Go where type- or interface-based dispatching is directly supported. In Python, however, there is no language support for type-based dispatching. You may occasionally see criticism of direct type-testing in Python, but in practice it's both extremely common and quite effective. It enables a function to have a high degree of generality, handling whatever datatypes are likely to come its way, aka "duck typing." Note the leading underscore on _ver_file; that is a conventional way of designating a "private" function (or method). While it can technically be called directly, it suggests that function is not intended for direct external consumption.


2019 update: Given recent updates in Python 3, for example that paths are now potentially stored as pathlib.Path objects not just str or bytes (3.4+), and that type hinting has gone from esoteric to mainstream (circa 3.6+, though still actively evolving), here's updated code that takes these advances into account:

from pathlib import Path
from typing import IO, Any, AnyStr, Union

Pathish = Union[AnyStr, Path]  # in lieu of yet-unimplemented PEP 519
FileSpec = Union[IO, Pathish]

def _ver_file(f: IO) -> Any:
    "Process file f"
    ...
    return result

def ver(fileobj: FileSpec) -> Any:
    "Process file (or file path) f"
    if isinstance(fileobj, (str, bytes, Path)):
        with open(fileobj, 'r') as f:
            return _ver_file(f)
    else:
        return _ver_file(fileobj)
  • 1
    Duck typing would test based on what you can do with the object, rather than what its type is. For example, trying to call read on something that might be file-like, or calling open(fileobj, 'r') and catching the TypeError if fileobj isn't a string. – user2357112 Nov 11 '14 at 0:01
  • You're arguing for duck typing in use. The example provides duck typing in effect--that is, users get the ver operation independent of type. It might also be possible to implement ver through duck typing, as you say. But generating then catching exceptions is slower than simple type inspection, and IMO does not yield any particular benefit (clarity, generality, etc.) In my experience, duck typing is awesome "in the large," but neutral to counterproductive "in the small." – Jonathan Eunice Nov 11 '14 at 1:05
  • 3
    No, what you're doing still isn't duck typing. A hasattr(fileobj, 'read') test would be duck typing; an isinstance(fileobj, str) test is not. Here's an example of the difference: the isinstance test fails with unicode filenames, since u'adsf.txt' isn't a str. You've tested for a too specific type. A duck typing test, whether based on calling open or some hypothetical does_this_object_represent_a_filename function, wouldn't have that problem. – user2357112 Nov 11 '14 at 2:06
  • 1
    If the code were production code rather than an explanatory example, I also wouldn't have that problem, because I wouldn't use is_instance(x, str) but rather something like is_instance(x, string_types), with string_types properly set for proper operation across PY2 and PY3. Given something that quacks like a string, ver would react properly; given something that quacks like a file, the same. To a user of ver, there would be no difference--except that the type inspection implementation would run faster. Duck purists: feel free to disagree. – Jonathan Eunice Nov 11 '14 at 2:23
4

If you pass the file name around instead of the file handle then there is no guarantee that the second file is the same file as the first one when it is opened; this can lead to correctness bugs and security holes.

  • 1
    True. But that must be counterbalanced with another tradeoff: If you pass around a file handle, all readers must coordinate their accesses to the file, because each is likely to move the "current file position." – Jonathan Eunice Nov 11 '14 at 16:00
  • @JonathanEunice: Coordinate in what sense? All they need to do is set the file position to be wherever they want it to be. – Mehrdad Nov 11 '14 at 19:32
  • 1
    If there are multiple entities reading the file, there may be dependencies. One may need to start where another one left off (or in a place defined by data read by a preceding read). Also, readers may be running in different threads, opening up other coordination cans of worms. Passed-around file objects become exposed global state, with all the issues (as well as benefits) that entails. – Jonathan Eunice Nov 11 '14 at 19:37
  • 1
    It's not passing around the file path that's the key. It's having one function (or class, method, or other locus of control) assume responsibility for "the complete processing of the file." If file accesses are encapsulated somewhere, then you do not need to pass around mutable global state like open file handles. – Jonathan Eunice Nov 11 '14 at 19:45
  • 1
    Well, we can agree to disagree then. I am saying there is a decided downside to designs that glibly pass around mutable global state. There are some advantages, too. Thus, a "tradeoff." Designs that pass file paths often do I/O in one fell swoop, in an encapsulated way. I see that as an advantageous coupling. YMMV. – Jonathan Eunice Nov 11 '14 at 20:14
1

This is about ownership and the responsibility to close the file. You can pass on a stream or file handle or whatever thingy that should be closed/disposed at some point to another method, as long as you make sure it is clear who owns it and certain it will be closed by the owner when you are done. This typically involves a try-finally construct or the disposable pattern.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.