Re: [matplotlib-devel] png error with git master, is it just me?

2013-04-08 Thread Michael Droettboom 
Hey there,

Just getting to this after a vacation.  I think I was stymied by the 
fact that the Fedora package for pycxx provides a pkg-config file, 
whereas, as you point out, it is not upstream.

I'll put my further comments on the issue.

Cheers,
Mike

On 04/05/2013 11:23 PM, Fernando Perez wrote:
> On Fri, Apr 5, 2013 at 2:51 PM, Julian Taylor
>  wrote:
>
>> I filed an issue:
>> https://github.com/matplotlib/matplotlib/issues/1884
> Go figure, it was an actual bug! Well, thanks a lot for tracking it down :)
>
> I guess for now I'll just remove pycxx-dev from my system.
> Fortunately I don't need it for anything else.  But it means that
> right now, mpl can't be built on a system with the matplotlib
> build-deps loaded, which is a bummer.
>
> Cheers,
>
> f
>
> --
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


--
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Re: [matplotlib-devel] Bug in imsave

2013-04-08 Thread Benjamin Root 
On Sun, Apr 7, 2013 at 4:39 AM, gary ruben  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
[email protected]
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Re: [matplotlib-devel] Bug in imsave

2013-04-08 Thread Michael Droettboom 

On 04/08/2013 02:38 PM, Benjamin Root wrote:




On Sun, Apr 7, 2013 at 4:39 AM, gary ruben > 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Re: [matplotlib-devel] Bug in imsave

2013-04-08 Thread gary ruben 
ceil doesn't work for me
I tried
figsize = [np.ceil(x / float(dpi)) for x in (arr.shape[1], arr.shape[0])]
and it fails on my second test when dpi=2 and i=25
result=26x26

Gary


On 9 April 2013 07:03, Michael Droettboom  wrote:

>  On 04/08/2013 02:38 PM, Benjamin Root wrote:
>
>
>
>
> On Sun, Apr 7, 2013 at 4:39 AM, gary ruben  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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
>
>
--
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter___
Matplotlib-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel