Force closing PIL image files #972

Merged
merged 2 commits into from Jul 18, 2012

Projects

None yet

4 participants

@cgohlke
cgohlke commented Jun 29, 2012

This fixes some ResourceWarning: unclosed file warnings under Python 3.2

@pelson pelson commented on the diff Jun 30, 2012
lib/matplotlib/image.py
@@ -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"""
@pelson
pelson Jun 30, 2012 Matplotlib Developers member

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?

@cgohlke
cgohlke Jun 30, 2012

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.

@pelson
pelson Jun 30, 2012 Matplotlib Developers member

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

@cgohlke
cgohlke Jun 30, 2012

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.

@pelson pelson commented on an outdated diff Jun 30, 2012
lib/matplotlib/image.py
"""try to load the image with PIL or return None"""
- 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:
@pelson
pelson Jun 30, 2012 Matplotlib Developers member

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

@pelson
Member
pelson commented Jun 30, 2012

+1

@mdboom
Member
mdboom commented Jul 1, 2012

Looks good.

@WeatherGod
Member

what is holding up this PR?

@pelson
Member
pelson commented Jul 18, 2012

Nothing.

@pelson pelson merged commit 7a6897e into matplotlib:master Jul 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment