-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Allow access of signed 16-bit TIFF pixel data as NumPy arrays #2577
Conversation
Only small additions needed to access the pixel data as numpy arrays were made. Code to convert to other pixel types such as 32-bit integers are not included.
@tomgoddard Please can you add unit tests? |
This is discussed in #2228 |
I added conversion code between I;16S and modes I and L and added I;16S to the test_mode_i16.py unit test.
|
I added tests of the I;16S mode a week ago. Is there more I need to do to move this along? |
In general I against adding modes with For example, in case of I think we should accept this. |
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.
Please check, that you are using 4 spaces as indention everywhere.
libImaging/Convert.c
Outdated
int x; | ||
INT32* out = (INT32*) out_; | ||
for (x = 0; x < xsize; x++, in += 2) | ||
*out++ = (INT16)(in[0] + ((int) in[1] << 8)); |
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.
wrong indention
libImaging/Convert.c
Outdated
int x; | ||
for (x = 0; x < xsize; x++, in += 2) | ||
if (in[1] & 0x80) | ||
*out++ = 0; /* Negative -> 0 */ |
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.
wrong indention
I've been looking at this and adding more tests, access, and whatnot. I'm now leaning against this approach. Prior to this PR, I think the core issue is in the tiff
The last to elements are ( Also, mirroring this is the
Though, we do optimistically offer a bunch of rawmodes in the
Also, there's this one other mode, which is broken as well:
There's got to be a better solution to this than adding a supported mode which doesn't work with any of the Image operations that we do. Option 1 is just to fix the TiffImagePlugin to not advertise modes we don't support. We could losslessly read an Alternately, we could do something along the lines of an adapter that provides an I'm hesitant to add a lightly supported mode like I;16S that has such a potential for numerical error. |
For my uses being able to access I;16S TIFF files as int32 arrays would be fine.
Does the comment "We have had rawmodes in Unpack.c for these formats to unpack to an I mode the whole time.” mean there is some way to read I;16S data currently? The following code using Image.convert(‘I’) on an I;16S TIFF image gives an error in Pillow 4.2.1
path = '/Users/goddard/Downloads/test_i16s.tif'
from PIL import Image
i = Image.open(path).convert(‘I')
File "test.py", line 3, in <module>
i = Image.open(path).convert('I')
File "/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/PIL/Image.py", line 860, in convert
self.load()
File "/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/PIL/TiffImagePlugin.py", line 1018, in load
return super(TiffImageFile, self).load()
File "/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/PIL/ImageFile.py", line 188, in load
self.load_prepare()
File "/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/PIL/ImageFile.py", line 261, in load_prepare
self.im = Image.core.new(self.mode, self.size)
ValueError: unrecognized mode
|
These lines in TiffImagePlugin should be changed to make it work:
|
(I'll note that it's likely completely untested, but I believe that there might be a way to trigger that code in currently shipping Pillow by doing raw conversions from a buffer or bytes.) |
Thanks! This two line change solves the problem in my software reading 16-bit signed TIFF microscopy images. With this change numpy.array(image) produces 32-bit int arrays. It would be better if 16-bit int arrays were directly produced but 32-bit int is a perfectly workable solution. The advantage of 16-bit is that these can be giga-byte size multiimage stacks (3-dimensional microscopy) so efficiently reading into a native 16-bit int array is preferred for performance reasons.
|
Small additions to access I;16S TIFF pixel data as numpy arrays.
Code to convert to other pixel types such as 32-bit integers is not included.
This pull request doesn't fix all the deficiencies in handling 16-bit signed TIFF data but it is a start. I use this in a patched Pillow version in a visualization package ChimeraX for light microscopy data.
Partial fix to issue #861
Related to closed issue #273 addressed by merged pull request #347
Changes proposed in this pull request:
The current Pillow master branch and 4.1.1 release gives an error on the seek and a zero dimensional array when trying to get the pixels. This pull request gives the correct results. Here is a multipage I;16S tiff example file.
Here is the error using from the seek() call using Pillow 4.1.1
and the incorrect result of the numpy array call in Pillow 4.1.1