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

Provide an error message when user gives incorrect arguments to Image.crop() #5957

Closed
alexwlchan opened this issue Jan 13, 2022 · 3 comments · Fixed by #5972
Closed

Provide an error message when user gives incorrect arguments to Image.crop() #5957

alexwlchan opened this issue Jan 13, 2022 · 3 comments · Fixed by #5972

Comments

@alexwlchan
Copy link
Contributor

What did you do?

I called Image.crop(), but I got confused about what the arguments are meant to be.

It's meant to be a (left, upper, right, lower)-tuple, but I supplied a (left, upper, width, height)-tuple instead. The error message I got was:

SystemError: tile cannot extend outside image

which took me a while to figure out, because all the crops looked like they were inside the image – I think possibly this is a side effect of width being lower than left, and height being lower than upper?

What did you expect to happen?

I'd get an error message explaining my mistake.
Maybe the crop() function could throw an error like:

ValueError: crop bounds should be a (left, upper, right, lower)-tuple, but right=100 was less than left=50

because I don't think cropping ever makes sense if right > left and lower > upper aren't satisfied.

What actually happened?

I got confused.

What are your OS, Python and Pillow versions?

  • OS: macOS 10.15.7
  • Python: Python 3.9.9
  • Pillow: Pillow==9.0.0

Steps to reproduce

This is a 100×100 pixel image:

gradient

from PIL import Image

im = Image.open("gradient.png")
print(im.size)

# What I think I'm doing:
# left=0, upper=0, width=25, height=25
#
# This works, because box[2] > box[0] and box[3] > box[1]
crop1 = im.crop(box=(0, 0, 25, 25))
crop1.save("crop1.png")

# What I think I'm doing:
# left=50, upper=50, width=25, height=25
#
# What I should actually have done:
# left=50, upper=50, right=75, upper=75
crop2 = im.crop(box=(50, 50, 25, 25))
crop2.save("crop2.png")

In case it's helpful, here's the full stack trace:

Stack trace
Traceback (most recent call last):
  File "/Users/alexwlchan/Library/Python/3.9/lib/python/site-packages/PIL/ImageFile.py", line 495, in _save
    fh = fp.fileno()
AttributeError: '_idat' object has no attribute 'fileno'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./example.py", line 19, in <module>
    crop2.save("crop2.png")
  File "/Users/alexwlchan/Library/Python/3.9/lib/python/site-packages/PIL/Image.py", line 2212, in save
    save_handler(self, fp, filename)
  File "/Users/alexwlchan/Library/Python/3.9/lib/python/site-packages/PIL/PngImagePlugin.py", line 1348, in _save
    ImageFile._save(im, _idat(fp, chunk), [("zip", (0, 0) + im.size, 0, rawmode)])
  File "/Users/alexwlchan/Library/Python/3.9/lib/python/site-packages/PIL/ImageFile.py", line 503, in _save
    e.setimage(im.im, b)
SystemError: tile cannot extend outside image
@radarhere
Copy link
Member

To be clear, the error message you're seeing isn't coming from crop().
Your crop() operation results in a (0, 0) image.
The error is coming when you try to save that (0, 0) image.

So I would say that you're not asking for us to change an error message - you're asking us to add an error message when performing a... "negative"... crop(), rather than just happily returning a (0, 0) image.

@alexwlchan
Copy link
Contributor Author

you're asking us to add an error message when performing a... "negative"... crop(), rather than just happily returning a (0, 0) image

Ah, I see – yes, that's what I was thinking.

@radarhere radarhere changed the title Provide a better error message when user gives incorrect arguments to Image.crop() Provide an error message when user gives incorrect arguments to Image.crop() Jan 14, 2022
@radarhere
Copy link
Member

See what you think of PR #5972

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.

2 participants