On 04/08/2013 02:38 PM, Benjamin Root wrote:
On Sun, Apr 7, 2013 at 4:39 AM, gary ruben <gary.ru...@gmail.com
<mailto:gary.ru...@gmail.com>> wrote:
Hi, I haven't looked at this list for a long time, so hi to all
the active devs.
I just came across a problem in the image.py imsave() function.
Problem:
At an ipython --pylab prompt, typing
a = rand(29,29)
imsave('test.png', a)
incorrectly generates a 28x28 pixel image.
This happens for several array sizes (I initially saw it for 226x226).
Line 1272 of image.py is
figsize = [x / float(dpi) for x in (arr.shape[1], arr.shape[0])]
figsize interacts with the bounding box code somehow to create
off-by-one results for certain values of array shape.
Possible solution:
To fix this properly, I think the float cast should be removed and
integer-based math should be used, but I thought that since it was
rounding down in the cases I saw, a change to the following should
fix this:
figsize = [(x + 0.5) / float(dpi) for x in (arr.shape[1],
arr.shape[0])]
and indeed this seems to work.
To verify this, I ran the following brute-force test of all image
sizes up to 1024x1024 at some common dpi values:
import matplotlib.pyplot as plt
import numpy as np
import os
for dpi in [75, 100, 150, 300, 600, 1200]:
for i in range(1,1025):
print dpi, i,
im = np.random.random((i,i))
fname = 'test{:03d}.png'.format(i)
plt.imsave(fname, im, dpi=dpi)
im = plt.imread(fname)[:,:,0]
print im.shape
assert im.shape == (i, i)
os.remove(fname)
and everything passed.
For completeness I also explored the dpi-space with these loop
ranges and the above loop body:
for dpi in range(1, 101):
for i in range(25, 36):
...
and all was still well.
Workaround:
For the moment I've set dpi=1 in my call to imsave which
effectively reverts its behaviour to the original imsave code
prior to the introduction of the dpi parameter.
I think this testing is rigorous enough to make this change.
If you agree, has anyone got time to change this, or shall I do a
pull request when I have time?
thanks,
Gary
Good catch on this. So you are saying that this bug was introduced
relatively recently with the dpi kwarg? I would suggest you make a PR
so that this doesn't get lost (or at the very least file a bug
report). As for the fix itself, off the top of my head, wouldn't
math.ceil() do what we want? I prefer to be explicit in any intents
rather than just simply (x + 0.5) / float(dpi). As for the test, I
would wonder if some sort of restricted version of that test might not
be good to do? I am thinking about 10 different sized plots and we
wouldn't even need to have a the assert check or file removal test, as
the image comparison framework would throw an exception when
attempting to compare two images of different sizes (I think...).
I think this bug goes back at least to hash 17192518, by me in May
2010. The suggested fix (probably using ceil to be more explicit as you
suggest) seems right.
As for the test, I think just `imsave` to a buffer, and loading that
buffer back in with `imread` and asserting the sizes are the same should
be sufficient.
Mike
------------------------------------------------------------------------------
Minimize network downtime and maximize team effectiveness.
Reduce network management and security costs.Learn how to hire
the most talented Cisco Certified professionals. Visit the
Employer Resources Portal
http://www.cisco.com/web/learning/employer_resources/index.html
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
