Repository: python-pillow/Pillow
Pull request: Fix ImageChops documentation
Many methods were incorrectly documented as requiring mode "1". The remaining
ones require *both* images to be mode "1".

Documentation only, [ci skip]

Fixes #2571

Problem statement:
ImageChops: logical_and produces "Image has wrong mode" with 2 RGB images
### What did you do?
I tried to apply logical_and between two RGB images

### What did you expect to happen?
AND function pixel-by-pixel, color-by-color

### What actually happened?
"Image has wrong mode" message. According to the docs, 
"At this time, most channel operations are only implemented for 8-bit images (e.g. “L” and “RGB”)."

### What versions of Pillow and Python are you using?
Python 3.6.1, PIL 1.1.7

```python
from PIL import Image, ImageChops

def test_chops():
    img = Image.new("RGB", (100, 100), 0xffffff)
    new_img = ImageChops.logical_and(img, img)


def main():
    test_chops()
    return 0

main()
```

Hints from issue discussion:
Full traceback with Pillow 4.1.1 and Python 2.7:
```
Traceback (most recent call last):
  File "1.py", line 12, in <module>
    main()
  File "1.py", line 9, in main
    test_chops()
  File "1.py", line 5, in test_chops
    new_img = ImageChops.logical_and(img, img)
  File "/usr/local/lib/python2.7/site-packages/PIL/ImageChops.py", line 216, in logical_and
    return image1._new(image1.im.chop_and(image2.im))
ValueError: image has wrong mode
```
The error is coming from [here](https://github.com/python-pillow/Pillow/blob/master/libImaging/Chops.c#L65-L67):
```c
    if (!im1 || !im2 || im1->type != IMAGING_TYPE_UINT8 ||
        (mode != NULL && (strcmp(im1->mode, "1") || strcmp(im2->mode, "1"))))
	return (Imaging) ImagingError_ModeError();
```
So the mode must be "1". This code runs:
```python
from PIL import Image, ImageChops

def test_chops():
    img = Image.new("1", (100, 100), 0xffffff)
    new_img = ImageChops.logical_and(img, img)


def main():
    test_chops()
    return 0

main()
```
Thanks for looking into this! 

I didn't have the time to test it, but I suspected that might have been the problem. I guess, at first sight, mode '1' might seem the only mode appropiate for binary operations, but it could be useful for masking colors in RGB(A) images too. Another reason to enable L or RGB would be to avoid having to convert the image mode for some operations, while needing RGB for other.

Anyway, as included in the original post, the ImageChops docs seem to imply that the operations  are mainly meant for L or RGB mode images. so I'd like to suggest either updating the docs, or the functions. If there's interest (except from me) in including the ops for RGB/L I might try to implement them.
PRs welcome for both! 

Docs can be found here: https://github.com/python-pillow/Pillow/tree/master/docs
I don't think that 'and' works for multi bit images. The relevant operation there is multiply.  (Unless we're looking at bitwise and, but that's not an operation that I think is a standard channel op.)

Documentation is probably the proper approach here. 
similar with `ImageChops.difference` when using modes `I` or `F`. I guess these are supposed to support higher bit rate and floats.
>I guess, at first sight, mode '1' might seem the only mode appropiate for binary operations, but it could be useful for masking colors in RGB(A) images too.

Indeed I found this issue when I tried to generate a mask for an 'L' source Image by just using logical AND with an image that was all 255 (I would expect the result to be 0 where the source is 0 and 255 else since thats what <code> x and 255</code> would evaluate to.)

Also I tried to combine two 'L' images with an logical or, and expected the result to be the first image value when it is not 0, second image value unless that is 0 as well, 0 if both pixels are 0.

_The 'L' images are in fact supposed to be 'P' Images with 0 as alpha so that operation would make sense for me._

I would not have tried that, had the documentation indicated that this attempt is futile. 
I've created PR #3522 to add documentation about this.
I believe the documentation fix is incorrect (overzealous), after having scratched my head over it for a couple hours today. For instance, it now claims that ImageChops.add() requires one of the arguments to be mode "1".

https://github.com/python-pillow/Pillow/pull/3522 states that one of the images must have mode "1". I think this is universally incorrect: most of the time, it's untrue (the function wouldn't be very useful with that requirement), and in the cases where it's required, it's required for *both* arguments.

This can be validated by checking the underlying C library: The CHOP and CHOP2 macros take a 2nd argument "mode", which is NULL for most functions but "1" for And/Or/Xor. "mode" is passed through to create() where we see the following:

```c
    if (!im1 || !im2 || im1->type != IMAGING_TYPE_UINT8 ||
        (mode != NULL && (strcmp(im1->mode, "1") || strcmp(im2->mode, "1"))))
        return (Imaging) ImagingError_ModeError();
```

It's important to remember that strcmp() returns *0* on equality. So the condition is saying that if the mode is not null and *either* of the image's modes is not "1", then return an error. But if the mode is NULL, we don't care about the mode (although we still care about the type, but strangely only the type of the first image). The string value of the "mode" argument doesn't actually matter, since the strcmp is hardcoded to "1".

Here's Python test code validating the above code sleuthing:

```python
#!/usr/bin/python3

from PIL import Image, ImageChops

print('Version: ' + Image.__version__)
im1 = Image.new('RGB', (100, 100), (55, 9, 233))
im2 = Image.new('RGB', (100, 100), (10, 44, 72))

print('im1: ' + str(im1.getpixel((0, 0))))
print('im2: ' + str(im2.getpixel((0, 0))))
print()

def test_func(func):
  out = func(im1, im2)
  print('%s: %s' % (func.__name__, out.getpixel((0, 0))))

for x in [ImageChops.lighter, ImageChops.darker, ImageChops.difference,
    ImageChops.multiply, ImageChops.screen, ImageChops.add,
    ImageChops.subtract, ImageChops.add_modulo, ImageChops.subtract_modulo]:
  test_func(x)

print()

# From here out things *do* require mode '1', from *both* arguments.
im3 = Image.new('L', (100, 100), 9)
im4 = Image.new('1', (100, 100), 1)

def test_func2(func):
  try:
    func(im1, im4)
    raise RuntimeError("Shouldn't get here!")
  except ValueError:
    pass
  try:
    func(im3, im4)
    raise RuntimeError("Shouldn't get here either!")
  except ValueError:
    pass
  out = func(im4, im4)
  print('%s: %s' % (func.__name__, out.getpixel((0, 0))))

for x in [ImageChops.logical_and, ImageChops.logical_or,
    ImageChops.logical_xor]:
  test_func2(x)
```

Current output from that, for me:

```
Version: 7.0.0
im1: (55, 9, 233)
im2: (10, 44, 72)

lighter: (55, 44, 233)
darker: (10, 9, 72)
difference: (45, 35, 161)
multiply: (2, 1, 65)
screen: (63, 52, 240)
add: (65, 53, 255)
subtract: (45, 0, 161)
add_modulo: (65, 53, 49)
subtract_modulo: (45, 221, 161)

logical_and: 255
logical_or: 255
logical_xor: 0
```

Commit message: Fix ImageChops documentation.

Many methods were incorrectly documented as requriring mode "1". The remaining
ones require *both* images to be mode "1".

Documentation only, [ci skip]

Patch to review:
diff --git a/src/PIL/ImageChops.py b/src/PIL/ImageChops.py
index 2d13b529fef..904df484e86 100644
--- a/src/PIL/ImageChops.py
+++ b/src/PIL/ImageChops.py
@@ -54,7 +54,7 @@ def invert(image):
 def lighter(image1, image2):
     """
     Compares the two images, pixel by pixel, and returns a new image containing
-    the lighter values. At least one of the images must have mode "1".
+    the lighter values.
 
     .. code-block:: python
 
@@ -71,7 +71,7 @@ def lighter(image1, image2):
 def darker(image1, image2):
     """
     Compares the two images, pixel by pixel, and returns a new image containing
-    the darker values. At least one of the images must have mode "1".
+    the darker values.
 
     .. code-block:: python
 
@@ -88,7 +88,7 @@ def darker(image1, image2):
 def difference(image1, image2):
     """
     Returns the absolute value of the pixel-by-pixel difference between the two
-    images. At least one of the images must have mode "1".
+    images.
 
     .. code-block:: python
 
@@ -107,8 +107,7 @@ def multiply(image1, image2):
     Superimposes two images on top of each other.
 
     If you multiply an image with a solid black image, the result is black. If
-    you multiply with a solid white image, the image is unaffected. At least
-    one of the images must have mode "1".
+    you multiply with a solid white image, the image is unaffected.
 
     .. code-block:: python
 
@@ -124,8 +123,7 @@ def multiply(image1, image2):
 
 def screen(image1, image2):
     """
-    Superimposes two inverted images on top of each other. At least one of the
-    images must have mode "1".
+    Superimposes two inverted images on top of each other.
 
     .. code-block:: python
 
@@ -179,7 +177,6 @@ def add(image1, image2, scale=1.0, offset=0):
     """
     Adds two images, dividing the result by scale and adding the
     offset. If omitted, scale defaults to 1.0, and offset to 0.0.
-    At least one of the images must have mode "1".
 
     .. code-block:: python
 
@@ -196,8 +193,7 @@ def add(image1, image2, scale=1.0, offset=0):
 def subtract(image1, image2, scale=1.0, offset=0):
     """
     Subtracts two images, dividing the result by scale and adding the offset.
-    If omitted, scale defaults to 1.0, and offset to 0.0. At least one of the
-    images must have mode "1".
+    If omitted, scale defaults to 1.0, and offset to 0.0.
 
     .. code-block:: python
 
@@ -212,8 +208,7 @@ def subtract(image1, image2, scale=1.0, offset=0):
 
 
 def add_modulo(image1, image2):
-    """Add two images, without clipping the result. At least one of the images
-    must have mode "1".
+    """Add two images, without clipping the result.
 
     .. code-block:: python
 
@@ -228,8 +223,7 @@ def add_modulo(image1, image2):
 
 
 def subtract_modulo(image1, image2):
-    """Subtract two images, without clipping the result. At least one of the
-    images must have mode "1".
+    """Subtract two images, without clipping the result.
 
     .. code-block:: python
 
@@ -244,8 +238,10 @@ def subtract_modulo(image1, image2):
 
 
 def logical_and(image1, image2):
-    """Logical AND between two images. At least one of the images must have
-    mode "1".
+    """Logical AND between two images.
+
+    Both of the images must have mode "1". For an AND in RGB mode, use a
+    multiply() by a black-and-white mask.
 
     .. code-block:: python
 
@@ -260,8 +256,9 @@ def logical_and(image1, image2):
 
 
 def logical_or(image1, image2):
-    """Logical OR between two images. At least one of the images must have
-    mode "1".
+    """Logical OR between two images.
+
+    Both of the images must have mode "1".
 
     .. code-block:: python
 
@@ -276,8 +273,9 @@ def logical_or(image1, image2):
 
 
 def logical_xor(image1, image2):
-    """Logical XOR between two images. At least one of the images must have
-    mode "1".
+    """Logical XOR between two images.
+
+    Both of the images must have mode "1".
 
     .. code-block:: python
 


Code review comments:
diff_hunk: @@ -244,8 +238,10 @@ def subtract_modulo(image1, image2):
 
 
 def logical_and(image1, image2):
-    """Logical AND between two images. At least one of the images must have
-    mode "1".
+    """Logical AND between two images.
+
+    Both of the images must have mode "1". For an AND in RGB mode, use a
+    multiply() by a black-and-white mask.
path: src/PIL/ImageChops.py
text: @user1:
```suggestion
    Both of the images must have mode "1". If you would like to perform a
    logical AND on an image with a mode other than "1", try
    :py:meth:`~PIL.ImageChops.multiply` instead, using a black-and-white mask
    as the second image.
```

Suggestion. This advice isn't exclusive to RGB.

The ':py:meth' part is a way to link the method in the docs. For an example, see the [close method docstring](https://github.com/python-pillow/Pillow/blob/a28b2ea11c5c7e63adb9b29d136d412c862f7328/src/PIL/Image.py#L590) and the [way it is rendered](https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.close).