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

Show warning when trying to save RGBA image as JPEG #2010

Merged
merged 2 commits into from Aug 25, 2016

Conversation

Projects
None yet
2 participants
@homm
Member

homm commented Jul 3, 2016

Fixes #2005

@homm homm added this to the 3.4.0 milestone Jul 3, 2016

@wiredfool

This comment has been minimized.

Show comment
Hide comment
@wiredfool

wiredfool Jul 4, 2016

Member

The problem here is that it's liable to break thumbnailing scripts that read PNGs, and this behavior dates from the PIL fork. (so it's well established). We'd be replacing something that does questionable things to the alpha channel with an exception.

Member

wiredfool commented Jul 4, 2016

The problem here is that it's liable to break thumbnailing scripts that read PNGs, and this behavior dates from the PIL fork. (so it's well established). We'd be replacing something that does questionable things to the alpha channel with an exception.

@homm

This comment has been minimized.

Show comment
Hide comment
@homm

homm Jul 4, 2016

Member

PNG have 16 base color formats. 6 of them fails PNG → JPEG conversation on master:

from PIL import Image
for fn in glob('../i/basn*.png'):
    try:
        Image.open(fn).save(fn + '.jpg')
    except IOError:
        print '>>>', fn

They are:

>>> ../i/basn0g16.png
>>> ../i/basn3p01.png
>>> ../i/basn3p02.png
>>> ../i/basn3p04.png
>>> ../i/basn3p08.png
>>> ../i/basn4a08.png

We can leave the alpha channel discarding for now, but I afraid this means that we leave it forever.

Member

homm commented Jul 4, 2016

PNG have 16 base color formats. 6 of them fails PNG → JPEG conversation on master:

from PIL import Image
for fn in glob('../i/basn*.png'):
    try:
        Image.open(fn).save(fn + '.jpg')
    except IOError:
        print '>>>', fn

They are:

>>> ../i/basn0g16.png
>>> ../i/basn3p01.png
>>> ../i/basn3p02.png
>>> ../i/basn3p04.png
>>> ../i/basn3p08.png
>>> ../i/basn4a08.png

We can leave the alpha channel discarding for now, but I afraid this means that we leave it forever.

@wiredfool

This comment has been minimized.

Show comment
Hide comment
@wiredfool

wiredfool Jul 5, 2016

Member

We can make it a specific deprecation warning, and anyone who wants to can register that as an exception. Then we can switch it over in 2-4 releases. (probably after stretch freezes, or April '17)

Member

wiredfool commented Jul 5, 2016

We can make it a specific deprecation warning, and anyone who wants to can register that as an exception. Then we can switch it over in 2-4 releases. (probably after stretch freezes, or April '17)

@homm

This comment has been minimized.

Show comment
Hide comment
@homm

homm Jul 5, 2016

Member

Ok, I'll add warning then.

Sorry, what is stretch freezes?

Member

homm commented Jul 5, 2016

Ok, I'll add warning then.

Sorry, what is stretch freezes?

@wiredfool

This comment has been minimized.

Show comment
Hide comment
@wiredfool

wiredfool Jul 5, 2016

Member

Debian Stretch, the next stable release. I'm leaning to include a deprecation warning in one iteration of Debian stable before making a hard error.

Member

wiredfool commented Jul 5, 2016

Debian Stretch, the next stable release. I'm leaning to include a deprecation warning in one iteration of Debian stable before making a hard error.

@homm

This comment has been minimized.

Show comment
Hide comment
@homm

homm Jul 5, 2016

Member

Ok, caught your point.

Member

homm commented Jul 5, 2016

Ok, caught your point.

@homm homm added the Do Not Merge label Jul 10, 2016

@homm homm changed the title from Do not discard alpha channel while saving JPEGs to Show warning when trying to save RGBA image as JPEG Jul 11, 2016

@homm homm removed the Do Not Merge label Aug 9, 2016

@homm

This comment has been minimized.

Show comment
Hide comment
@homm

homm Aug 9, 2016

Member

Turned the error to the warning.

Member

homm commented Aug 9, 2016

Turned the error to the warning.

@wiredfool wiredfool merged commit e980ca7 into python-pillow:master Aug 25, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 78.164%
Details
@wiredfool

This comment has been minimized.

Show comment
Hide comment
@wiredfool

wiredfool Aug 25, 2016

Member

This should probably get a release note as a future deprecation warning.

Member

wiredfool commented Aug 25, 2016

This should probably get a release note as a future deprecation warning.

@homm homm deleted the uploadcare:jpeg-raise-on-alpha branch Sep 19, 2016

hugovk added a commit to hugovk/Pillow that referenced this pull request May 27, 2017

hugovk added a commit to hugovk/Pillow that referenced this pull request May 27, 2017

hugovk added a commit to hugovk/Pillow that referenced this pull request May 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment