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

Bugfix: EPS thumbnail failed #619

Merged
merged 10 commits into from
May 24, 2014
Merged

Conversation

eliempje
Copy link
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
Contributor Author

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

@hugovk
Copy link
Member

hugovk commented Apr 12, 2014

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
Contributor Author

Do you know who to correct this?

@aclark4life
Copy link
Member

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

@eliempje
Copy link
Contributor Author

I will try. I get back to you.

@karstenw
Copy link

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
Copy link
Contributor Author

eliempje commented May 7, 2014

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
Copy link
Member

hugovk commented May 8, 2014

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

How can #302 be tested?

@eliempje
Copy link
Contributor Author

eliempje commented May 8, 2014

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

@eliempje
Copy link
Contributor Author

eliempje commented May 8, 2014

Thanks for adding tests.

Tests for EPS thumbnail bugfix (including thumbnail bugfix)
@eliempje
Copy link
Contributor Author

eliempje commented May 8, 2014

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

@hugovk
Copy link
Member

hugovk commented May 8, 2014

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
Copy link
Member

hugovk commented May 8, 2014

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
Copy link
Member

hugovk commented May 8, 2014

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

@wiredfool
Copy link
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
Copy link
Member

hugovk commented May 8, 2014

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

@aclark4life
Copy link
Member

True 😄 Just under 2 months

Fix tests so image2 is resized and thumbnailed
@eliempje
Copy link
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
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