Repository navigation
ImagePalette's palette argument expects different alignment since 8.3.0 #5595
Description
It seems that PIL.ImagePalette.ImagePalette's palette argument no longer expects a palette aligned by channels, but one color after the other, i.e. RGBRGBRGB rather than RRRGGGBBB as previously.
I assume this is an unintentional breaking change as it is neither specifically mentioned in the patch notes, nor was the documentation updated. I could not find any PR or bug report on this, either (unless I missed it - I am sorry if that's the case).
If this is a bug, I hope it can actually become a feature as I personally prefer the new ordering.
Example:
import PIL.Image
import PIL.ImagePalette
p = [0, 0, 0,
255, 0, 0,
0, 255, 0]
palette = PIL.ImagePalette.ImagePalette(mode='RGB', palette=p, size=len(p))
image = PIL.Image.new('P', (100, 100), 1) # fill image with second color in the palette
image.putpalette(palette)
image.show() # Gives a blue image on 8.2.0 and a red image since 8.3.0
assert image.getpalette()[:len(p)] == p, 'This assert succeeds in 8.3.0'Cheers!
- OS: Arch Linux
- Python: 3.9
- Pillow: 8.3.0+
Activity
radarhere commentedon Jul 9, 2021
Testing, I find this is the result of 1ee30de from #5552
With that particular change, I was interested in fixing a bug where the palette could break just because it was marked dirty. See this new test that was added there. It failed previously.
But you can see in something like 4d36fee, from earlier in #5552, that my intention was for RGBRGBRGB.
I've created PR #5599 to update the documentation. Let me know if there is anything else missing.
krukai commentedon Jul 9, 2021
Thank you for the quick reply! As far as I am aware, that is all.
But is it really a good idea to force a breaking change? I will now have to re-arrange my input palette depending on the pillow version installed, and I doubt I am the only one depending pillow.
As I said, I would love this new ordering to become a feature, but backwards compatibility should be honored, shouldn't it?
Cheers!
radarhere commentedon Jul 10, 2021
Pillow does value backwards compatibility, quite highly.
If this issue had been raised before 8.3.0, then this would have had a good chance of being addressed.
If this had even been raised before 8.3.1, then maybe.
If this issue had something like the 25 responses to 8.3.0 bug of #5571 that caused us to release 8.3.1, again, let's try and keep Pillow working for users, maybe we'll put together 8.3.2.
However, if we're talking about reverting the behaviour in 8.4.0, I'm not convinced that switching back after 3 months of different behaviour is helpful.
As to why the change happened in the first place, here is my personal feeling - ImagePalette has been broken.
Allow me to start with your code and go from there.
It sounds like maybe you're building a library with Pillow as a dependency, and you don't know what version of Pillow your users will have installed? If so, then perhaps using
putpalettewith an array directly will solve that problem for you, as it behaves the same way in 8.2.0 and 8.3.0.krukai commentedon Jul 11, 2021
Thank you very much for the detailed explanation and insight! I fully agree now that this is an inconsistency, first and foremost.
Frankly, I just chose the first way I came across to to reach my goal, and passing the array directly to
putpalette()will work fine for me.Thank you again for looking into this and working on pillow in general.
Cheers!
Edit: I figure once the documentation change is merged for good, this can be closed.