This is a quick potential one-character fix. But certainly longer, as it is better to write tests about it.
What happens?
I was reading this code:
(which was added in d522e0a)
|
elif len(source_palette) > 768: |
|
bands = 4 |
|
palette_mode = "RGBA" |
And I realized this is probably buggy. Because 3 * 256 = 768. But that code only detects RGBA if len(source_palette) > 768. It should have been >= instead of >. Probably.
What did you do?
I just read the code. I did not write any code (yet). I did not use the remap_palette function. And, in fact, I do not intend to use it in the near future. I just read the code and found a potential bug. Consider it a late code review. ;)
Proposed solution?
- Write down some tests using RGBA palette.
- Check if the tests fail. If my understanding is correct, the tests will fail.
- Replace
> with >=. The tests should now succeed.
Can someone from Pillow please investigate and check if this is the case?
Thanks! I hope my little bug report over here helps. :)
This is a quick potential one-character fix. But certainly longer, as it is better to write tests about it.
What happens?
I was reading this code:
(which was added in d522e0a)
Pillow/src/PIL/Image.py
Lines 2234 to 2236 in c8c74c8
And I realized this is probably buggy. Because
3 * 256 = 768. But that code only detects RGBAif len(source_palette) > 768. It should have been>=instead of>. Probably.What did you do?
I just read the code. I did not write any code (yet). I did not use the
remap_palettefunction. And, in fact, I do not intend to use it in the near future. I just read the code and found a potential bug. Consider it a late code review. ;)Proposed solution?
>with>=. The tests should now succeed.Can someone from Pillow please investigate and check if this is the case?
Thanks! I hope my little bug report over here helps. :)