Hi Tony,<br><br><div class="gmail_quote">On Sat, Oct 31, 2009 at 5:36 PM, Tony S Yu <span dir="ltr">&lt;<a href="mailto:tsyu80@gmail.com">tsyu80@gmail.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style="word-wrap: break-word;">Hi Ralf, <div><br></div><div>Thanks for the feedback.</div><div><br><div><div><div class="im"><div>On Oct 30, 2009, at 11:31 AM, Ralf Gommers wrote:</div><blockquote type="cite"><div class="gmail_quote">
<div>The images on the page you linked to can not be included I think (copyright), so could you generate two small palette images (one rgb, one grey, 10x10 pixels) for use in a test? (or ideally, write the test:))<br></div>
</div></blockquote><div><br></div></div>I couldn&#39;t easily find any palette images that weren&#39;t copyrighted; it never occurred to me just create them. Thanks.</div></div></div></div></blockquote><div><br>You&#39;re welcome. In addition to avoiding copyright issues, it is nice to have very small images. Otherwise the size of the scikit would be completely dominated by lots of large test images after a while.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style="word-wrap: break-word;"><div><div><div class="im"><div><br></div><div><blockquote type="cite">
<div class="gmail_quote"><div>
Also, I see you are already using git, so next time could you push your changes to github as described here <a href="http://stefanv.github.com/scikits.image/contribute.html#development-process" target="_blank">http://stefanv.github.com/scikits.image/contribute.html#development-process</a>, that would make it a bit faster to test.<br>
</div></div></blockquote></div><div><br></div></div><div>I was afraid I&#39;d have to learn git. ;)</div><div><br></div></div></div></div></blockquote><div>I trust it wasn&#39;t too painful, thanks for doing it. <br> <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style="word-wrap: break-word;"><div><div><div></div><div>I added the changes in three commits to: <a href="http://github.com/tonysyu/scikits.image" target="_blank">http://github.com/tonysyu/scikits.image</a></div>
<div>The first commit just adds the images, the second adds palette image handling + a test, and the third adds an additional test.</div><div><br></div><div>I wasn&#39;t sure how to test imread. I ended up checking that imread returns a 2D array for the grayscale palette image and a 3D array for the color palette image. Another possibility is to test the values of the array, but that seems fragile.</div>
<div><br></div></div></div></div></blockquote><div>This looks fine to me. Testing values would be good as well, and if you do it with assert_array_almost_equal from numpy.testing it should not be fragile.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style="word-wrap: break-word;"><div><div><div></div><div>The last commit adds a test of the function `palette_is_gray`. Unfortunately, the test is a bit ugly. First, I had to add an import of PIL in the test, since palette_is_gray works specifically on PIL palette images. Second, I had to add `palette_is_grayscale` to __all__ in pil_imread so that I could use the function in my test. (Python documentation suggests that __all__ only affects ``from &lt;&gt; import *``, but it seemed to prevent ``from <a href="http://scikits.image.io" target="_blank">scikits.image.io</a> import palette_is_grayscale``.)</div>
<div><br></div></div></div></div></blockquote><div>The PIL import is fine, but the try-except block is unnecessary since that is already done in imread.<br><br>Adding to __all__ is not necessary, you can import it from io.pil_imread.<br>
<br>I did notice that other imread tests fail already if PIL is not available. For this we do need an @pil_skip decorator I think. <br>  <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style="word-wrap: break-word;"><div><div><div></div><div>Let me know what you think of the tests. Also, should I do a pull request?</div></div></div></div></blockquote><div><br>Looks good overall. I made some minor changes reflecting the comments I made above: <a href="http://github.com/rgommers/scikits.image/commit/a6ad7415d7a43185cea898d3454e087b0b01f105">http://github.com/rgommers/scikits.image/commit/a6ad7415d7a43185cea898d3454e087b0b01f105</a><br>
<br>If you are happy with those, you can pull my palette branch over and send Stefan a pull request.<br><br>Cheers,<br>Ralf<br></div></div>