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.

Sign up for GitHub

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

Jump to bottom

Allow tuples with one item to give single color value in getink #4927

Merged
merged 3 commits into from Oct 12, 2020
Merged

Allow tuples with one item to give single color value in getink #4927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27c0747
Allow tuples with one item to give single color value in getink
radarhere Sep 20, 2020
e01081b
Improved error message
radarhere Sep 21, 2020
b304a13
Updated test
radarhere Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions Tests/test_image.py
View file
Open in desktop
Expand Up @@ -498,6 +498,12 @@ def test_storage_neg(self):
with pytest.raises(ValueError):
Image.core.fill("RGB", (2, -2), (0, 0, 0))

def test_one_item_tuple(self):
for mode in ("I", "F", "L"):
im = Image.new(mode, (100, 100), (5,))
px = im.load()
assert px[0, 0] == 5

def test_linear_gradient_wrong_mode(self):
# Arrange
wrong_mode = "RGB"
Expand Down
11 changes: 6 additions & 5 deletions Tests/test_image_access.py
View file
Open in desktop
Expand Up @@ -330,21 +330,22 @@ def test_p_putpixel_rgb_rgba(self):
class TestImagePutPixelError(AccessTest):
IMAGE_MODES1 = ["L", "LA", "RGB", "RGBA"]
IMAGE_MODES2 = ["I", "I;16", "BGR;15"]
INVALID_TYPES1 = ["foo", 1.0, None]
INVALID_TYPES2 = [*INVALID_TYPES1, (10,)]
INVALID_TYPES = ["foo", 1.0, None]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INVALID_TYPES = ["foo", 1.0, None]
INVALID_TYPES = ["foo", 1.0, None, ((10,),)]

Tuple can be accepted, but probably not tuple-in-tuple. Maybe test for that as well? Or is that too much of an edge-case?


@pytest.mark.parametrize("mode", IMAGE_MODES1)
def test_putpixel_type_error1(self, mode):
im = hopper(mode)
for v in self.INVALID_TYPES1:
for v in self.INVALID_TYPES:
with pytest.raises(TypeError, match="color must be int or tuple"):
im.putpixel((0, 0), v)

@pytest.mark.parametrize("mode", IMAGE_MODES2)
def test_putpixel_type_error2(self, mode):
im = hopper(mode)
for v in self.INVALID_TYPES2:
with pytest.raises(TypeError, match="color must be int"):
for v in self.INVALID_TYPES:
with pytest.raises(
TypeError, match="color must be int or single-element tuple"
):
im.putpixel((0, 0), v)

@pytest.mark.parametrize("mode", IMAGE_MODES1 + IMAGE_MODES2)
Expand Down
5 changes: 4 additions & 1 deletion src/_imaging.c
View file
Open in desktop
Expand Up @@ -518,6 +518,9 @@ getink(PyObject* color, Imaging im, char* ink)
be cast to either UINT8 or INT32 */

int rIsInt = 0;
if (PyTuple_Check(color) && PyTuple_Size(color) == 1) {
color = PyTuple_GetItem(color, 0);
}
Comment on lines +521 to +523
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this currently makes L mode accept tuple(tuple(10)) and RGB mode accept tuple(tuple(255,255,255)), which is probably not the intended behaviour.

Suggested change
if (PyTuple_Check(color) && PyTuple_Size(color) == 1) {
color = PyTuple_GetItem(color, 0);
}
/* IMAGING_TYPE_UINT8 unpacks tuples later */
if (im->type != IMAGING_TYPE_UINT8 && PyTuple_Check(color) && PyTuple_Size(color) == 1) {
color = PyTuple_GetItem(color, 0);
}

if (im->type == IMAGING_TYPE_UINT8 ||
im->type == IMAGING_TYPE_INT32 ||
im->type == IMAGING_TYPE_SPECIAL) {
Expand All @@ -533,7 +536,7 @@ getink(PyObject* color, Imaging im, char* ink)
return NULL;
}
} else {
PyErr_SetString(PyExc_TypeError, "color must be int");
PyErr_SetString(PyExc_TypeError, "color must be int or single-element tuple");
return NULL;
}
}
Expand Down