Should I close a stream (file-like object) passed into my object in the context manager __exit__() function?

StackOverflow https://stackoverflow.com/questions/17489731

Question

I have an object which I'd like to be able to use the with keyword on. I'm happy with the practicalities implementing context managers but I'm hitting my head up against a best practice kind of issue.

The object is a wrapper around a file. I'm planning for my object to be initialised either with a string (the path of the file) or with a file-like which can be dealt with directly (there's a possibility of files inside files - so I foresee a definite use case for this with BytesIO etc...)

So __init__looks something like this:

def __init__(self, file_to_process):
    if isinstance(file_to_process, str):
        self._underlying_stream = open(file_to_process, "rb") # it's the path to a file
    elif isinstance(file_to_process, io.IOBase):
        self._underlying_stream = file_to_process # its the file itself
    else:
         raise TypeError()

So my question is, is it best practice/acceptable/sensible to close that _underlying_stream in my __exit__() function? It totally makes sense when it was a path, but if it's a stream passed in, it strikes me as impolite at best and dangerous at worst to close self._underlying_stream - am I correct to be thinking that, and if so, is there a neat way around this?

(Note: I considered wrapping the stream coming in with a io.BufferedReader, but it turns out that closing that will also close the underlying stream...)

Was it helpful?

Solution

I would not close the underlying stream. Passing in an already open file object means the caller has taken responsibility of that object and closing that object on __exit__ would be extremely annoying, at best.

PIL does something similar, albeit not in a context manager. When passing in a filename, it'll close the fileobject after it completes reading the image data. It sets a boolean flag just for that. Pass in a file object instead and it'll read but not close.

I'd do the same here:

class Foo(object):
    _close_on_exit = False

    def __init__(self, file_to_process):
        if isinstance(file_to_process, str):
            self._underlying_stream = open(file_to_process, "rb") # it's the path to a file
            self._close_on_exit = True
        elif isinstance(file_to_process, io.IOBase):
            self._underlying_stream = file_to_process # its the file itself
        else:
             raise TypeError()

    def __exit__(self, exc_type, exc_value, traceback):
        if self._close_on_exit:
            self._underlying_stream.close()
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top