On Sun, Apr 7, 2013 at 4:39 AM, gary ruben <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...).
Cheers!
Ben Root
------------------------------------------------------------------------------
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
