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 convert_mode param when saving #3172

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

radarhere
Copy link
Member

Resolves #2663

@radarhere radarhere force-pushed the convert_mode branch 7 times, most recently from 9e29cef to 36797eb Compare June 16, 2018 03:52
@homm
Copy link
Member

homm commented Jun 30, 2018

We don't need to store a map of conversions for each file format. If format supports, for example, only RGB and image in CMYK, we always will do CMYK→ RGB conversion regardless of format. All we need to know is which modes are available for the desired file format. The convert_mode function should just convert from any of source modes to one of the available modes.

Moreover, we could add more internal modes from time to time without touching any plugins.

@radarhere
Copy link
Member Author

Okay, I've added a commit. See what you think

homm
homm previously requested changes Jun 30, 2018
src/PIL/Image.py Outdated Show resolved Hide resolved
src/PIL/Image.py Outdated Show resolved Hide resolved
src/PIL/Image.py Outdated Show resolved Hide resolved
src/PIL/Image.py Outdated Show resolved Hide resolved
@radarhere radarhere force-pushed the convert_mode branch 2 times, most recently from 6e00bc2 to fba0d01 Compare July 1, 2018 11:13
@hugovk hugovk dismissed homm’s stale review July 1, 2018 18:00

Updates made, new review welcome!

@hugovk hugovk requested a review from homm July 1, 2018 18:00
@radarhere radarhere force-pushed the convert_mode branch 2 times, most recently from ca023d2 to fb9d188 Compare October 5, 2018 11:23
@radarhere radarhere force-pushed the convert_mode branch 2 times, most recently from b9e90c7 to 1cd3c57 Compare March 30, 2019 11:26
@homm
Copy link
Member

homm commented May 12, 2019

In general, this is great. The only drawback for me is binding of supported modes for the save function via the parent's module attribute. In particular, this prevents different sets of supported modes in one module. Isn't better to explicitly assign the supported modes to the save function?

def _save(im, fp, filename):
   pass

_save._supported_modes = ['RGB', 'RGBA', 'P', 'I', 'LA', 'L', '1']

@radarhere radarhere force-pushed the convert_mode branch 3 times, most recently from 462e5de to 55650a3 Compare June 12, 2019 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert image to supported mode before saving
2 participants