Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force closing PIL image files #972

Merged
merged 2 commits into from Jul 18, 2012
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 14 additions & 6 deletions lib/matplotlib/image.py
Expand Up @@ -1185,12 +1185,20 @@ def imread(fname, format=None):
can be used with :func:`~matplotlib.pyplot.imshow`.
"""

def pilread():
def pilread(fname):
"""try to load the image with PIL or return None"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have extended the capability of this function (file handles are now supported), can you improve the doctoring a little. Perhaps the variable name fname is no longer appropriate - maybe something like img_file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the capability of the function is the same as before. It just works around another bug in PIL, which does not seem to close all the files it opens.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. but the variable naming statement still holds (fname could be a file handle or a filename).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it is a good idea to rename the variable. fname is used as an argument in the outer function. Renaming it there would break the API. Renaming it in pilread only would be confusing I think.

try: from PIL import Image
except ImportError: return None
image = Image.open( fname )
return pil_to_array(image)
try:
from PIL import Image
except ImportError:
return None
if cbook.is_string_like(fname):
# force close the file after reading the image
with open(fname, "rb") as fp:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fh is the traditional "file" handle/pointer variable name.

image = Image.open(fp)
return pil_to_array(image)
else:
image = Image.open(fname)
return pil_to_array(image)

handlers = {'png' :_png.read_png, }
if format is None:
Expand All @@ -1206,7 +1214,7 @@ def pilread():
ext = format

if ext not in handlers.iterkeys():
im = pilread()
im = pilread(fname)
if im is None:
raise ValueError('Only know how to handle extensions: %s; with PIL installed matplotlib can handle more images' % handlers.keys())
return im
Expand Down