Skip to content

Bugfix: EPS thumbnail failed - #619

Merged
wiredfool merged 10 commits into
python-pillow:masterfrom
eliempje:master
May 24, 2014
Merged

wiredfool merged 10 commits into
python-pillow:masterfrom
eliempje:master

Conversation

@eliempje

Copy link
Copy Markdown
Contributor

EPS thumbnail failed to resize correctly due to incorrect resolution argument (should be a function of the image size and bounding box). This is fixed in this commit.

EPS thumbnail failed to resize correctly due to incorrect resolution argument (should be a function of the image size and bounding box). This is fixed in this commit.
@eliempje

Copy link
Copy Markdown
Contributor Author

Why did the Travis CI build fail? I just changed a few lines of code.

@hugovk

hugovk commented Apr 12, 2014

Copy link
Copy Markdown
Member

This test failed:

Tests/test_file_eps.py:91: assert_image_similar(image1_scale2, image1_scale2_compare, 5) failed:
- average pixel value difference 10.8121 > epsilon 5.0000
Tests/test_file_eps.py:98: assert_image_similar(image2_scale2, image2_scale2_compare, 10) failed:
- average pixel value difference 18.9240 > epsilon 10.0000

Also, do you have a test that shows the bug fails before the fix, and passes with the fix?

@eliempje

Copy link
Copy Markdown
Contributor Author

Do you know who to correct this?

@aclark4life

Copy link
Copy Markdown
Member

@eliempje We need you to know how to fix it 😄

@eliempje

Copy link
Copy Markdown
Contributor Author

I will try. I get back to you.

@karstenw

Copy link
Copy Markdown

If you have a test that checks for image sizes and change the output imagesize, it has to fail.

I have a fork where eps files work for me. Not patch quality:
https://github.com/karstenw/Pillow/blob/master/PIL/EpsImagePlugin.py

aclark4life and others added 2 commits April 30, 2014 04:42
FIXED issue python-pillow#302: python-pillow#302
EPS file can have binary preview. Header is now also read binary.

Also fix for resizing EPS. Resolution is now 2 dimensional and dependend on bbox and size.
@eliempje

eliempje commented May 7, 2014

Copy link
Copy Markdown
Contributor Author

Test passed this time. This commit even adds another bugfix for #302
Could you merge now?

hugovk added a commit to hugovk/Pillow that referenced this pull request May 8, 2014
@hugovk

hugovk commented May 8, 2014

Copy link
Copy Markdown
Member

I've submitted resize and thumbnail tests to @eliempje's fork here: eliempje#2

How can #302 be tested?

@eliempje

eliempje commented May 8, 2014

Copy link
Copy Markdown
Contributor Author

make an eps with binary preview in the header (you could use ghostscript for that). I have one but its copyrighted.

@eliempje

eliempje commented May 8, 2014

Copy link
Copy Markdown
Contributor Author

Thanks for adding tests.

Tests for EPS thumbnail bugfix (including thumbnail bugfix)
@eliempje

eliempje commented May 8, 2014

Copy link
Copy Markdown
Contributor Author

Did mean ghostscript but photoshop (under windows). This will add a preview in the first few bytes.

@hugovk

hugovk commented May 8, 2014

Copy link
Copy Markdown
Member

You're welcome for the tests. Unfortunately the build failed: https://travis-ci.org/python-imaging/Pillow/builds/24725294

running test_file_eps ...
Tests/test_file_eps.py:132: assert_equal(image1.size, new_size) failed:
- got (100, 76), expected (100, 100)
Tests/test_file_eps.py:133: assert_equal(image2.size, new_size) failed:
- got (360, 252), expected (100, 100)

Actually, I just spotted a bug in the test. image1 is thumbnailed a second time, instead of image2. I've sent a PR to fix it (eliempje#3), but it only accounts for the second failure, not the first.

If you want to run just that test locally, call python Tests/test_file_eps.py, or python Tests/run.py for the whole set.

I don't have Photoshop, maybe someone else can create a test image.

@hugovk

hugovk commented May 8, 2014

Copy link
Copy Markdown
Member

Apologies, the reason it's failing is due to me not reading the docs properly. With the latest build, it fails like this:

Tests/test_file_eps.py:132: assert_equal(image1.size, new_size) failed:
- got (100, 76), expected (100, 100)
Tests/test_file_eps.py:133: assert_equal(image2.size, new_size) failed:
- got (100, 70), expected (100, 100)

https://travis-ci.org/hugovk/Pillow/jobs/24741008

But Image.thumbnail's docs say:

This method modifies the image to contain a thumbnail version of itself, no larger than the given size. This method calculates an appropriate thumbnail size to preserve the aspect of the image...

I'll fix the test properly and update the PR.

@hugovk

hugovk commented May 8, 2014

Copy link
Copy Markdown
Member

OK, eliempje#3 passes now (https://travis-ci.org/hugovk/Pillow/builds/24742214). Sorry for the confusion!

@wiredfool

Copy link
Copy Markdown
Member

@hugovk Thanks for picking up the slack and looking at this (and the other issues). I'm kind of snowed under right now, but I am paying enough attention to know that I'm way behind on review and merging.

@hugovk

hugovk commented May 8, 2014

Copy link
Copy Markdown
Member

@wiredfool No problem! Don't worry, there's months until the next release :)

@aclark4life

Copy link
Copy Markdown
Member

True 😄 Just under 2 months

Fix tests so image2 is resized and thumbnailed
@eliempje

Copy link
Copy Markdown
Contributor Author

Test added. Now ready for merge

wiredfool added a commit that referenced this pull request May 24, 2014
Bugfix: EPS thumbnail failed
@wiredfool
wiredfool merged commit 3f18e0a into python-pillow:master May 24, 2014
@wiredfool

Copy link
Copy Markdown
Member

Thanks everyone.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants