ImageOps.autocontrast: add mask parameter

[navneeth]

This is a draft of an enhancement to autocontrast that allows the specification of a mask to constrain the contrast stretching.
autocontrast(image, mask=a_mask)

Perhaps it Fixes # #3331.

Changes proposed in this pull request:

• adding an optional mask argument. if none is specified, behaviour defaults to how it was before.
• added tests in Tests/test_imageops.py
• also added tests for cutoff argument, where the tuple case was not being checked.

This is my first contribution to pillow. So do let me know if this suffices.

[hugovk]

Thanks for the PR!

Here's some suggestions.

To fix the lint, you can do this:

pip install -U black
black .

pip install -U isort==4.3.21
isort -y

Or manually apply the diff from https://github.com/python-pillow/Pillow/pull/4843/checks#step:8:57.

And we should probably add release notes for this. But we can follow up with that later :)

[hugovk]

This test asserts that an autocontrasted image with a mask is different an autocontrasted image without a mask. Okay.

But for all we know the masked image could be a blank black image. Could you add something to actually test for correctness of the masked output?

[navneeth]

Hmm. That will require some deeper thought. What you are suggesting is a functionality
test for the autocontrast method.
I noticed that the baseline autocontrast did not have such a functional test in place.

1. So I guess the test would apply to both ?
2. I would likely have to add a custom test image for such a test. Any guidelines on this ?

How about we close this PR and follow up with the functional test in another PR ?

[hugovk]

We could test an arbitrary pixel is something expected. For example:

diff --git a/Tests/test_imageops.py b/Tests/test_imageops.py
index 3d5223d9..c5ad670a 100644
--- a/Tests/test_imageops.py
+++ b/Tests/test_imageops.py
@@ -342,3 +342,5 @@ def test_auto_contrast_mask_real_input():
         result_nomask = ImageOps.autocontrast(img)

         assert result_nomask != result
+        assert result.getpixel((20, 30)) == (239, 239, 242)
+        assert result_nomask.getpixel((20, 30)) == (132, 118, 96)

No need to close and create a new PR, we can update this one.

[navneeth]

Just added code to test for explicit pixel value in the test. Rather than look for a pixel value it compares the stats (median) of the autocontrast (with mask) and autocontrast (without mask). This logic draws from the theory of autocontrast and should scale well. PR 01aeaa4

[hugovk]

Same comment here about testing for correctness.

[navneeth]

Thank you for the thorough and sensible review. I genuinely appreciate what you do.
cheers.
Just submitted an updated PR with the changes.

[hugovk]

We could test an arbitrary pixel is something expected. For example:

diff --git a/Tests/test_imageops.py b/Tests/test_imageops.py
index 3d5223d9..c5ad670a 100644
--- a/Tests/test_imageops.py
+++ b/Tests/test_imageops.py
@@ -342,3 +342,5 @@ def test_auto_contrast_mask_real_input():
         result_nomask = ImageOps.autocontrast(img)

         assert result_nomask != result
+        assert result.getpixel((20, 30)) == (239, 239, 242)
+        assert result_nomask.getpixel((20, 30)) == (132, 118, 96)

No need to close and create a new PR, we can update this one.

[navneeth]

Updated the PR with code to test for explicit pixel value in the test. Rather than look for a pixel value it compares the stats (median) of the autocontrast (with mask) and autocontrast (without mask). This logic draws from the theory of autocontrast and should scale well. Please see PR 2d3a841

[hugovk]

I've updated this PR with release notes:

• https://pillow--4843.org.readthedocs.build/en/4843/releasenotes/8.0.0.html#api-additions

[hugovk]

Thank you!