Skip to content
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

Added rawmode argument to Image.getpalette() #6061

Merged
merged 6 commits into from Feb 20, 2022

Conversation

radarhere
Copy link
Member

Resolves #6059

At the moment, getpalette() only returns the palette in RGB mode. This PR adds a rawmode argument that allows the user to specify the mode that the palette shoud be returned as.

I've also allowed the rawmode to be None, in case the user would like the palette returned without transformation.

@CookiePLMonster
Copy link

CookiePLMonster commented Feb 18, 2022

Would it make sense to also allow converting via a .tobytes(encoder, **args) call, much like it is done for Images?

@radarhere
Copy link
Member Author

Just for clarity - you've talking about ImagePalette.tobytes(), and you're suggesting that users be able to call ImagePalette.tobytes(rawmode) (encoders are for images, not palettes).

Bear with me as I try and put this into a coherent thought. I don't think there's an inherit pull for Image.tobytes() and ImagePalette.tobytes() to mirror each other. For one thing, the first argument of Image.tobytes() is "encoder" as you say, which is not something that applies to an ImagePalette. For another, these aren't the only tobytes() methods - there's also ImageCmsProfile.tobytes(), Dib.tobytes(), ImageFileDirectory_v2.tobytes() and Exif.tobytes().

After this PR, If you wanted to convert the data from an ImagePalette to a particular mode, you could

im.putpalette(palette)
im.getpalette(rawmode)

I would think of ImagePalette as being for the analysis and modification of a palette.

I can imagine one wanting to

  1. Load an image
  2. Modify the palette data in ImagePalette
  3. Attach the modified palette to the image proper through load() (or one of the many other methods that call load() internally)
  4. Convert the palette data to a certain mode, because you want to write it to file, alongside the image data.

But are you saying you have a scenario where you would like to skip step 3? So you have a situation where you would like to load an image, modify the palette, take a copy of the palette for external purposes, and no longer use the image with it's modified palette?

@CookiePLMonster
Copy link

Fair enough - I only suggested .tobytes() for consistency of the API, but your point of encoders not applying to palettes makes sense. I think the current approach is fine then.

But are you saying you have a scenario where you would like to skip step 3? So you have a situation where you would like to load an image, modify the palette, take a copy of the palette for external purposes, and no longer use the image with it's modified palette?

Not sure, I imagine that if there is a need for this the user may simply deep copy the palette. But to be precise, my idea was more along the lines of:

  1. Load image
  2. Get a palette as RGBA regardless of whether the image has alpha or not
  3. Write results to file

However, sounds like a modified getpalette() satisfies that requirement just fine too.

@radarhere radarhere changed the title Added rawmode argument to getpalette() Added rawmode argument to Image.getpalette() Feb 19, 2022
@hugovk hugovk merged commit b803b7c into python-pillow:main Feb 20, 2022
@hugovk
Copy link
Member

hugovk commented Feb 20, 2022

Thanks!

@radarhere radarhere deleted the getpalette branch February 20, 2022 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rawmode argument to Image.getpalette()
3 participants