-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix imread issues mentioned on matplotlib-devel #616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Other possible options for handling 16 bit images:
|
I like the idea of pil_to_array returning a 16 bit array when the input image is 16-bit. It allows the caller to deal with downsampling to 8-bit in the most appropriate way. We can update the docstring to indicate that the dtype of the output depends on the input file or some such. I think it's reasonable to make small backward-incompatible changes like this on master (just add to the changelog). While we're at it, can we add a test for this? Showing that a 16-bit input image is correctly returned as a 16-bit array? |
img = plt.imread(os.path.join(os.path.dirname(__file__), | ||
'baseline_images/test_image/uint16.tif')) | ||
assert (img.dtype == np.uint16) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be turned into an image comparison test to ensure that the fields of the image are correctly read in, endianness works etc. Or if that's not practical, maybe assert about a few of the data values?
I have one small note about testing. This doesn't (I believe) change the fact that PNG images read through the _png extension (i.e. not through PIL) are still converted to 8-bit. We should fix that as well. |
I don't think all of these changes can go into v1.1.x. Not flipping PIL images or allowing 16 bit images might break existing applications. The _png extension should be fixed in another PR. It is currently broken in master (test_png .py fails). |
I agree this shouldn't be on 1.1.x, but it should get merged into master. I've added an issue #915 to track adding 16-bit support to the _png extension. As far as I can tell, the I think the following would make this mergeable:
|
@cgohlke -- it looks like your fork of matplotlib has been removed, making it impossible for me to review this pull request. Any idea what might have happened? |
I was able to get the diff manually from here: https://github.com/matplotlib/matplotlib/pull/616.diff so all is not lost -- I should be able to verify and merge this by hand. |
Merged into master |
Thanks. My matplotlib fork got corrupted. |
http://sourceforge.net/mailarchive/message.php?msg_id=28520067