Pillow: PIL.Image.fromarray fails with uint16 arrays

[dgua]

Sorry if this isn't the right place.

When I try PIL.Image.fromarray with a uint16 array, it fails. Looking at the source code, uint16 is not supported. What would it take to add this data type?

[radarhere]

This is the right place. Welcome to the official home of Pillow, where bug reports and contributions are accepted. In this case, you're asking for an enhancement.

As with many enhancements, it's a question of hoping that someone knowledge takes an interest and is willing to figure out the change. Or if you are so inclined, you are more than welcome to put it together and then create a pull request for review.

Would it be possible for you to provide some sample code? I understand what you're asking, I'd just be interested in having a short program to experiment against. While I'm likely not the right person for this one, I would be keen in checking out the problem at hand.

[dgua]

On Nov 1, 2015, at 4:55:46, Andrew Murray [email protected] wrote:

This is the right place. Welcome to the official home of Pillow, where bug reports and contributions are accepted. In this case, you're asking for an enhancement.

As with many enhancements, it's a question of hoping that someone knowledge takes an interest and is willing to figure out the change. Or if you are so inclined, you are more than welcome to put it together and then create a pull request for review.

Would it be possible for you to provide some sample code? I understand what you're asking, I'd just be interested in having a short program to experiment against. While I'm likely not the right person for this one, I would be keen in checking out the problem at hand.

—
Reply to this email directly or view it on GitHub.

OK, here is the code (Python):

from PIL import Image

img_data is a np.array that contains the image (dtype=int16)

im = Image.fromarray(np.flipud(img_data))

This works

im.save('test.tiff’)

Now, ImageJ only supports uint16, not int16. So:

img_data2 = (img_data+32768).astype(dtype=np.uint16)
im2 = Image.fromarray(np.flipud(img_data))

This generates an error

I am using the Anaconda distribution with Python 2.7.

Davide

[mairsbw]

This problem still exists. I'm working with uint16 TIFFs for a remote sensing/GIS application. I can read the uint16 data from an image just fine, but can't write it back. Very unexpected that you can't roundtrip uint16 data with Pillow's Image module.

@radarhere If you could point me in the right direction I can likely code up this missing functionality as I would think it'd be trivial with the rest of the support available in this library.

@dgua I believe matplotlib and pylibtiff can work around this problem for now (see this source)

[mairsbw]

So saving a single-channel uint16 TIFF image doesn't work with matplotlib as it likes to save everything as 4-channel RGBA images.

[mairsbw]

Re: the above, see here

[radarhere]

Are you asking how to contribute, or for a hint as to which files to look at?

If contributing, then have a look at https://github.com/python-pillow/Pillow/blob/master/.github/CONTRIBUTING.md. Click on the Fork button at https://github.com/python-pillow/Pillow, then run git clone https://github.com/mairsbw/Pillow.git on your local machine. Make your changes, commit and push them back to origin. Once done, come back here and make a new pull request - https://github.com/python-pillow/Pillow/pull/new/master

[mairsbw]

Which files to look at. I've actually already contributed to this repo way back in the day and am familiar with that process.

[wiredfool]

It may work if the array is converted into a 32 bit datatype first. While there is support for reading int16 files, there isn't support in pillow for storage of anything but 8 or 32 bpp images.

[radarhere]

In trying to help @mairsbw, I'm having a difficult time determining where the relevant code is, after realising that this thread doesn't actually say what the error is.

That said, I'm guessing that the issue lies at https://github.com/python-pillow/Pillow/blob/master/PIL/Image.py#L2204

[wiredfool]

You're looking in the right place. It looks like it should be supported so we're at the point of looking and seeing what the error is and debugging from there.

[mairsbw]

Sorry about that, @radarhere, I thought this issue explained the error. The relevant error is the TypeError: Cannot handle this data type at Image.py:2168. From adding some debugging output I see that the KeyError raised is (1, 1), '<u2'). This image should be a greyscale (single-channel) TIFF with uint16 data, so that seems correct.

If you modify Image.py to add a mapping for the type found in the Uint16 that I'm using you can get it to support writing back TIFFs, but only as uint8 values, not uint16. So looks like there are necessary changes in the packing functions.

If you dig into Unpack.c you can see where the relevant 16-bit greyscale reading is supported. Pack.c has a similar section.

I have a pending pull request where we can continue the discussion of how to go about it.

[mairsbw]

In case references aren't sent out in notifications, I've added a PR w/ a test that fixes this in #1819.

[wiredfool]

Having gone through this and the numpy tests that we have in Tests/test_numpy.py

1) We're not testing potentially troublesome values that would indicate signed/unsigned errors or potential overflow. All of the values we're testing are small integers.
2) The signed conversions using the unsigned integer unpacking functions make me want to take a better look at them. particularly:

    ((1, 1), "<i2"): ("I", "I;16"),
    ((1, 1), ">i2"): ("I", "I;16B"),

I suspect, but have not tested, that this should actually be:

    ((1, 1), "<u2"): ("I", "I;16"),
    ((1, 1), ">u2"): ("I", "I;16B"),
    ((1, 1), "<i2"): ("I", "I;16S"),
    ((1, 1), ">i2"): ("I", "I;16BS"),

3) Similarly, the 32bit conversions appear to be a mix of signed/unsigned.

[mairsbw]

So are there specific changes you have in mind for the Tests/test_numpy.py code that you're working on or could suggest for me to implement? Sounds like they just need to be expanded to cover a wider range of conversions to make sure everything's correct. I can take a look through that today if that's where you think we should start.

[wiredfool]

Specific tests I'd look for are tests for values that are likely to expose issues, either as signed/unsigned or little/big endian. So, for int16, values like -1 and 0xEFFF, 0xEF00, for signed items, and 0, 0xFFFF and 0xFF00 for unsigned. They should have the same numeric value when set in numpy and when retrieved with the pixel access functions. (-1 exposes int/uint problems, the FF/00 bitmasks will show off issues with endianess, at least once we get a bigendian processor fired up again to test on. (gotta dig out my g4 mini... )(or when redhat runs their tests))

And heck, for good measure, it should work the other way too. Right now we're testing with values that are either range(100), 0, or whatever hopper(mode) winds up with, which are less than challenging.
(note that there are more conversions from Pillow -> numpy than the reverse. They're defined in _MODE_CONV, where the numpy->pillow ones are in _fromarry_typemap.)

[mairsbw]

So should I be writing the tests such that they:

  # Generate a numpy array with the specific datatype and desired pixel values
  # Create an Image object from those pixels using Image.fromarray
  # Use _test_img_equals_nparray() to check that the image is correct

And go through a range of pixel values and datatypes that should be supported?

It seems like the Image.fromarray() tests should be removed from test_numpy.py file and put into test_image_fromarray.py to be consistent with other tests. The numpy.array() tests can stay in tests_numpy.py. Then test_file_tiff.py can deal with the roundtripping tests and things like that. Does that seem like the correct organization?

[wiredfool]

That's a good outline of what to do.

Any tests that require numpy should be in test_numpy.py, so that they can be easily skipped if numpy is not installed. And since numpy is pretty much the only way we have to create an array for the array instance, and it's the way that most people who use that interface are accessing Pillow, it's better in test_numpy.py rather than renaming to test_image_fromarray.py

[mairsbw]

@wiredfool See #1824 for the initial work to clarify the image support for this. There shouldn't be anything "wrong" with those tests as they merely demonstrate the existing support of the library. The next step is to resolve these tests with the proper datatype mapping as you've alluded to earlier.

[mairsbw]

@wiredfool I believe you mean the following is what it should be:

diff --git a/PIL/Image.py b/PIL/Image.py
index c0515d3..dca76bb 100644
--- a/PIL/Image.py
+++ b/PIL/Image.py
@@ -2207,10 +2207,14 @@ _fromarray_typemap = {
     # ((1, 1), "|b1"): ("1", "1"), # broken
     ((1, 1), "|u1"): ("L", "L"),
     ((1, 1), "|i1"): ("I", "I;8"),
-    ((1, 1), "<i2"): ("I", "I;16"),
-    ((1, 1), ">i2"): ("I", "I;16B"),
-    ((1, 1), "<i4"): ("I", "I;32"),
-    ((1, 1), ">i4"): ("I", "I;32B"),
+    ((1, 1), "<u2"): ("I", "I;16"),
+    ((1, 1), ">u2"): ("I", "I;16B"),
+    ((1, 1), "<i2"): ("I", "I;16S"),
+    ((1, 1), ">i2"): ("I", "I;16BS"),
+    ((1, 1), "<u4"): ("I", "I;32"),
+    ((1, 1), ">u4"): ("I", "I;32B"),
+    ((1, 1), "<i4"): ("I", "I;32S"),
+    ((1, 1), ">i4"): ("I", "I;32BS"),
     ((1, 1), "<f4"): ("F", "F;32F"),
     ((1, 1), ">f4"): ("F", "F;32BF"),
     ((1, 1), "<f8"): ("F", "F;64F"),
diff --git a/Tests/test_numpy.py b/Tests/test_numpy.py
index 29323b4..4265012 100644
--- a/Tests/test_numpy.py
+++ b/Tests/test_numpy.py
@@ -55,19 +55,28 @@ class TestNumpy(PillowTestCase):
         self.assert_image(to_image(numpy.int8), "I", TestNumpy.TEST_IMAGE_SIZE)

         # Check non-fixed-size integer types
-        self.assertRaises(TypeError, lambda: to_image(numpy.uint))
+        self.assert_image(to_image(numpy.uint), "I", TestNumpy.TEST_IMAGE_SIZE)
         self.assert_image(to_image(numpy.int), "I", TestNumpy.TEST_IMAGE_SIZE)

         # Check 16-bit integer formats
         if Image._ENDIAN == '<':
-            self.assert_image(to_image(numpy.uint16), "I;16L", TestNumpy.TEST_IMAGE_SIZE)
+            self.assert_image(to_image(numpy.uint16), "I;16", TestNumpy.TEST_IMAGE_SIZE)
         else:
             self.assert_image(to_image(numpy.uint16), "I;16B", TestNumpy.TEST_IMAGE_SIZE)
-        self.assertRaises(TypeError, lambda: to_image(numpy.int16))
+        if Image._ENDIAN == '<':
+            self.assert_image(to_image(numpy.int16), "I;16S", TestNumpy.TEST_IMAGE_SIZE)
+        else:
+            self.assert_image(to_image(numpy.int16), "I;16BS", TestNumpy.TEST_IMAGE_SIZE)

         # Check 32-bit integer formats
-        self.assertRaises(TypeError, lambda: to_image(numpy.uint32))
-        self.assert_image(to_image(numpy.int32), "I", TestNumpy.TEST_IMAGE_SIZE)
+        if Image._ENDIAN == '<':
+            self.assert_image(to_image(numpy.uint32), "I;32", TestNumpy.TEST_IMAGE_SIZE)
+        else:
+            self.assert_image(to_image(numpy.uint32), "I;32B", TestNumpy.TEST_IMAGE_SIZE)
+        if Image._ENDIAN == '<':
+            self.assert_image(to_image(numpy.int32), "I;32S", TestNumpy.TEST_IMAGE_SIZE)
+        else:
+            self.assert_image(to_image(numpy.int32), "I;32BS", TestNumpy.TEST_IMAGE_SIZE)

         # Check 64-bit integer formats
         self.assertRaises(TypeError, lambda: to_image(numpy.uint64))

Note that this fails for int16LE images with these changes, though I think the tests are correct.

[wiredfool]

That's pretty much what I was thinking. Note that I;16L is an alias of I;16, I think it was added by mistake when I thought that I;16 was native format.

There are two possible issues that I see with it:
1) We're (likely) going to be changing the representation of an int16 from numpy -> pillow to use the signed representation, so the bytes in I32 storage are going to be different than they were for negative values. The integer values will be correct though.
2) The conversion from uint32->I;32 is potentially lossy, unless we just bitblat it in. But then we don't have the same signedness behavior with an int16 as an int32.

[mairsbw]

So are you saying that all data read into Pillow is implicitly stored as an int32 array? Is that something that needs to change for this support to be implemented?

Now to move this implementation forward, what would be the next steps? I'm happy to keep driving this forward, but as I discussed in #1824, the machinery within Pillow is insanely complicated for dealing with different datatypes. I think it'd be wise to build up a corpus of tests that should work but currently do not regarding uint16 imagery and then to patch the system to fix those tests.

[wiredfool]

It's... .complicated. I've got it all in my head now, but I don't think it's in the docs. It'll take brain time to produce though.

There are actually 3 (or 4) pointers, as defined in Imaging.h:

struct ImagingMemoryInstance {

    /* Format */
    char mode[IMAGING_MODE_LENGTH]; /* Band names ("1", "L", "P", "RGB", "RGBA", "CMYK", "YCbCr", "BGR;xy") */
    int type;       /* Data type (IMAGING_TYPE_*) */
    int depth;      /* Depth (ignored in this version) */
    int bands;      /* Number of bands (1, 2, 3, or 4) */
    int xsize;      /* Image dimension. */
    int ysize;

    /* Colour palette (for "P" images only) */
    ImagingPalette palette;

    /* Data pointers */
    UINT8 **image8; /* Set for 8-bit images (pixelsize=1). */
    INT32 **image32;    /* Set for 32-bit images (pixelsize=4). */

    /* Internals */
    char **image;   /* Actual raster data. */
    char *block;    /* Set if data is allocated in a single block. */

    int pixelsize;  /* Size of a pixel, in bytes (1, 2 or 4) */
    int linesize;   /* Size of a line, in bytes (xsize * pixelsize) */

    /* Virtual methods */
    void (*destroy)(Imaging im);
};

The only one that is guaranteed to be set is **image, which is an array of pointers to row data.

The rough sequence is:

• Image file is opened
• Each of the ImagePlugin _accept functions have a chance to look at the first few bytes to determine if they should attempt to open the file
• The *ImagePlugin._open method is called giving the image plugin a chance to read more of the image and determine if it still wants to consider it a valid image of it's particular type. If it does, it passes back a tile definition which includes a decoder and an image size.
• If there is a successful _open call, at some point later *ImagePlugin._load may be called on the image, which runs the decoder producing a set of bytes in a raw mode. This is where things like compression are handled, but the output of the decoder is not necessarily what we're storing in our internal structures.
• The image is unpacked (Unpack.c) from the raw mode (e.g. I16;BS) into a storage (Storage.c) mode (I).
• It's now possible to operate on the image (e.g. crop, pixel access, etc)

[mairsbw]

So this issue has kind of veered off topic. As of right now tests for this functionality are in #1824.

So I've addressed the first 3 in #1839 for opening a uint16 TIFF. But maybe we should keep this discussion there or create a new tracking issue for uint16 multi-page TIFF support as there are now many issues addressing that support.

[radarhere]

I'll note here that #1824 has been replaced by #1984, and that new PR has been merged.

[fe-codes]

img = Image.fromarray(np.zeros((10,10,3),'uint16'),'I;16')
Image.blend(img,img,0.5)

This piece of code will raise an error:

ValueError: image has wrong mode

[radarhere]

Looking at the code, the error is occurring because the first image is not uint8. So, for an example workaround, you could -

from PIL import Image
import numpy as np
im = Image.fromarray(np.zeros((10, 10, 3), 'uint16'), 'I;16')
im = im.convert("RGB")
Image.blend(im, im, 0.5)

If you would like this looked into further, please create a new issue.

[fe-codes]

Looking at the code, the error is occurring because the first image is not uint8. So, for an example workaround, you could -

from PIL import Image
import numpy as np
im = Image.fromarray(np.zeros((10, 10, 3), 'uint16'), 'I;16')
im = im.convert("RGB")
Image.blend(im, im, 0.5)

If you would like this looked into further, please create a new issue.

Thanks for replying! I am actually working with uint16 CT images, so I am not sure whether the conversion will cause a precision loss.