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

getpixel() should work with list of length 2 #7352

Closed
cipri-tom opened this issue Aug 24, 2023 · 6 comments · Fixed by #7355
Closed

getpixel() should work with list of length 2 #7352

cipri-tom opened this issue Aug 24, 2023 · 6 comments · Fixed by #7355

Comments

@cipri-tom
Copy link

cipri-tom commented Aug 24, 2023

What did you do?

img.getpixel([473, 653])

What did you expect to happen?

the pixel at that position is returned

What actually happened?

An unexpected error message: TypeError: argument must be sequence of length 2

But the argument is a sequence of length 2

What are your OS, Python and Pillow versions?

  • OS: Ubuntu 18.04
  • Python: 3.9
  • Pillow: '9.4.0'
img = Image.open('test.png')
img.getpixel([473, 653])
@hugovk
Copy link
Member

hugovk commented Aug 24, 2023

It's expecting a tuple not a list:

https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.getpixel

Pillow/src/_imaging.c

Lines 1149 to 1151 in 643a52a

if (!PyTuple_Check(xy) || PyTuple_GET_SIZE(xy) != 2) {
goto badarg;
}

Pillow/src/_imaging.c

Lines 1183 to 1185 in 643a52a

badarg:
PyErr_SetString(PyExc_TypeError, "argument must be sequence of length 2");
return -1;

@homm
Copy link
Member

homm commented Aug 25, 2023

It's expecting a tuple not a list:

But the question is why it can't be a list? "argument must be sequence of length 2", list is also a sequence. So behavior or message should be changed.

@cipri-tom
Copy link
Author

cipri-tom commented Aug 25, 2023

Exactly! Thanks, @homm

Of course, I would much prefer it it could take any sequence of 2 integers, so we could pass lists, numpy arrays, etc, as long as they have 2 integers.

Would probably have been easier if it took 2 integer arguments, but I suppose there's no changing the interface now

@Yay295
Copy link
Contributor

Yay295 commented Aug 25, 2023

I think that could be done actually. Something like this:

def getPixel(self, x, y=None):
    if y is None:
        xy = tuple(x)
    else:
        xy = (x, y)

@radarhere
Copy link
Member

I've created PR #7355 to allow img.getpixel([473, 653]).

It does not allow im.getpixel(473, 563). In a discussion about trying to clarify things after a confusing error message, it seems to me that "either an argument of sequence of length 2 or two arguments of single integer" is not a step towards clarity. But if someone else would like to create a PR for that change, they are free to do so and see it if receives approval from others.

@cipri-tom
Copy link
Author

Thank you for such quick decision and acting !

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 a pull request may close this issue.

5 participants