16

I want to write a function that accepts either a path as a string or a file object. So far I have:

def awesome_parse(path_or_file):
    if isinstance(path_or_file, basestring):
        f = open(path_or_file, 'rb')
    else:
        f = path_or_file
    with f as f:
        return do_stuff(f)

where do_stuff takes an open file object.

Is there a better way to do this? Does with f as f: have any repercussions?

Thanks!

0

2 Answers 2

20

The odd thing about your code is that if it is passed an open file, it will close it. This isn't good. Whatever code opened the file should be responsible for closing it. This makes the function a bit more complex though:

def awesome_parse(path_or_file):
    if isinstance(path_or_file, basestring):
        f = file_to_close = open(path_or_file, 'rb')
    else:
        f = path_or_file
        file_to_close = None
    try:
        return do_stuff(f)
    finally:
        if file_to_close:
            file_to_close.close()

You can abstract this away by writing your own context manager:

@contextlib.contextmanager
def awesome_open(path_or_file):
    if isinstance(path_or_file, basestring):
        f = file_to_close = open(path_or_file, 'rb')
    else:
        f = path_or_file
        file_to_close = None

    try:
        yield f
    finally:
        if file_to_close:
            file_to_close.close()

def awesome_parse(path_or_file):
    with awesome_open(path_or_file) as f:
        return do_stuff(f)
Sign up to request clarification or add additional context in comments.

3 Comments

Oh... good catch with it closing the file when it shouldn't! I definitely don't want that to happen. Thanks!
Shouldn't yield be yield f?
What exactly does the @contextlib.contextmanager do? Why do I get an AttributeError: __exit__ without it? Thanks!
6

You could do:

def awesome_parse(do_stuff):
    """Decorator to open a filename passed to a function
       that requires an open file object"""
    def parse_path_or_file(path_or_file):
        """Use a ternary expression to either open the file from the filename
           or just pass the extant file object on through"""
        with (open(path_or_file, 'rb') 
               if isinstance(path_or_file, basestring) 
                else path_or_file) as f:
            return do_stuff(f)
    return parse_path_or_file

And then when you declare any function that does stuff on an open file object:

@awesome_parse
def do_things(open_file_object):
    """This will always get an open file object even if passed a string"""
    pass

@awesome_parse
def do_stuff(open_file_object):
    """So will this"""
    pass

Edit 2: More detailed info on the decorator.

3 Comments

Ah, yeah, sorry. I just changed it to awesome_parse to not be confused with super.
It appears as though your answer falls into the same trap that @Ned Batchelder's answer addresses (closing the file prematurely). I do, however like the decorator idea, so I'll be using a bit of your idea as well!
Yeah, it does. Use his @contextlib.contextmanager structure in parse_path_or_file() and you'll be fine.

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.