802: handle EXIF orientations 3, 6, and 8 when cropping photos

[parhamr]

Status

Ready for review

Description of Changes

Fixes #802.

Changes proposed in this pull request:

• If an image has EXIF orientation, rotate it “upright” before applying the user-provided crop

Screenshots

The first two photos for Haywood BUTZ are crops of an image having EXIF orientation values; this proves it works because the F is upright. The last photo (cartoon) has no EXIF orientation and it appears upright, as expected.

---

Here’s what happens with the same image files against the develop branch: (the F is erroneously rotated 90º counter-clockwise)

---

Tests and linting

• I have rebased my changes on current develop

• pytests pass in the development environment on my local machine

• flake8 checks pass

[parhamr]

I’m mixed on writing tests for this. On one hand, it would ensure this works, does not break other things, and does not regress; I agree it’s a best practice. On the other hand, I’m not that comfortable with Python and I’m unsure how to write the tests for this case. Help?!

This page has some useful test images having EXIF orientation values: https://magnushoff.com/articles/jpeg-orientation/

These files have rotations 3, 6, and 8, which this PR covers:

[parhamr]

These loggers might be useless. They don’t really provide any actionable information and they expose a behavior the user would otherwise assume “just works.”

[dismantl]

I don't see the detriment to keeping them. It would be interesting for admins to see how often we run across these EXIF rotation issues.

[dismantl]

Any reason not to bump the Pillow dependency version instead?

[parhamr]

I’ll try that, too. I was a bit hesitant to mess with dependencies.

[dismantl]

I’m mixed on writing tests for this. On one hand, it would ensure this works, does not break other things, and does not regress; I agree it’s a best practice. On the other hand, I’m not that comfortable with Python and I’m unsure how to write the tests for this case. Help?!

This page has some useful test images having EXIF orientation values: https://magnushoff.com/articles/jpeg-orientation/

These files have rotations 3, 6, and 8, which this PR covers:

I would suggest adding those sample 'F' images to OpenOversight/app/static/images and write tests similar to those in test_image_tagging.py. The test_user_can_add_tag() and test_featured_tag_replaces_others() tests are good examples of how different pieces of the image tagging flow can be mocked. I like the idea of using those 'F' sample images for testing because when you submit to main.label_data it actually does the crop and saves a new image in raw_images. So the test could then compare the sha256 hash values of the newly cropped images (raw_images.hash_img) to see if they match the pre-computed hashes of those same images cropped but also rotated correctly. Does that make sense?

[parhamr]

@dismantl thanks for the feedback! I was also thinking the hashes of the image bytes would be useful for testing. I’ll poke around.

Additionally — is there an automated test runner for this code? I finally decided to speed up my test runs by specifying individual files in the -v tests/ part of the Makefile (https://github.com/lucyparsons/OpenOversight/blob/develop/Makefile#L41-L50), but that wasn’t quite as productive as I had hoped.

[parhamr]

I’m going to rework this on Pillow >= 6.0

[dismantl]

@dismantl thanks for the feedback! I was also thinking the hashes of the image bytes would be useful for testing. I’ll poke around.

Additionally — is there an automated test runner for this code? I finally decided to speed up my test runs by specifying individual files in the -v tests/ part of the Makefile (https://github.com/lucyparsons/OpenOversight/blob/develop/Makefile#L41-L50), but that wasn’t quite as productive as I had hoped.

@parhamr Yes, make test should run all the tests in docker, and you can also run individual tests or individual test modules by running name=<test_or_module> make test.