ImagePalette: loading a translucent RGBA color into an RGB palette will break the palette

[jsbueno]

This is a minor thing:

In PIL/ImagePalette.py, we have

Pillow/src/PIL/ImagePalette.py

Lines 118 to 119 in 243402e

	if len(color) == 4 and color[3] == 255:
	color = color[:3]

And then, this color is consumed in line 151, always assuming it is 3-bytes long (and having 4 would make colors from that point on "out of sync") . I was about to send a small PR raising value error in the unchecked case:

   if len(color) == 4 and color[3] < 255:
          raise ValueError(...)

But then, why not just truncate the alpha away, and always use the RGB value if an RGBA color is added to an RGB palette?
Can we do that?

(the sole purpose of this issue is to ask this question - I should submit fix + test afterwards)

[radarhere]

For context, the code to handle opaque RGBA colors was added in #5552.

I think that raising a ValueError is the better solution. If Pillow just accepted the input, this could surprise the user later when they discover that the RGBA color isn't really there.

[jsbueno]

Oh, -
i did the ValueError thing - but it looks like event he code in texts expect to be able to insert translucent colors,
and get the alpha rounded.

Tests/test_image_access.py::TestCffi::test_p_putpixel_rgb_rgba[P] FAILED [ 31%]

=================================== FAILURES ===================================
_____________________ TestCffi.test_p_putpixel_rgb_rgba[P] _____________________

self = <Tests.test_image_access.TestCffi object at 0x00007fb00ce8b8a0>
mode = 'P'

    @pytest.mark.parametrize("mode", ("P", "PA"))
    def test_p_putpixel_rgb_rgba(self, mode):
        for color in [(255, 0, 0), (255, 0, 0, 127)]:
            im = Image.new(mode, (1, 1))
            access = PyAccess.new(im, False)
>           access.putpixel((0, 0), color)

Tests/test_image_access.py:351: 
(...)
E                       ValueError: RGB ImagePalette can't handle non-opaque RGBA colors

So - do we use the RGB components?
Or should the failing tests be changed?

[radarhere]

The error you're hitting is when the tests try to set an RGBA value for a P image. The test was changed to use a translucent value in #6504, which was a PR about allowing RGB and RGBA values for PA images. Setting a translucent value for a P image was a side-effect, not the intention. So I'm happy to change the test.

I've created jsbueno#2