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

Return boolean from ImageMath lambda_eval and unsafe_eval comparison operations #8227

Closed
wants to merge 4 commits into from

Conversation

radarhere
Copy link
Member

from PIL import Image, ImageMath
A = Image.new("L", (1, 1))
print(ImageMath.lambda_eval(lambda args: args["A"] == args["A"], A=A))

currently gives <PIL.Image.Image image mode=I size=1x1 at 0x1047906E0>

This PR suggests changing the output to a more meaningful True.

==, !=, <, <=, >, >=, equal() and notequal() would all now return boolean results.

@hugovk
Copy link
Member

hugovk commented Oct 11, 2024

Is this a breaking change?

@radarhere
Copy link
Member Author

For ImageMath expressions that involve equality or comparison checks, yes.

Here are some alternative ways forward.

  1. Keep things as they are. The return type is _Operand, and we can type hint it as such and use a # type: ignore[override] to prevent mypy saying
error: Return type "_Operand" of "__eq__" incompatible with return type "bool" in supertype "object"
  1. We could made this breaking change only for lambda_eval() and unsafe_eval(). It would still be a breaking change for those methods (added in 10.3.0), but eval() would stay the same until it is removed in Pillow 12. Not a perfect solution, but an improvement.

@hugovk
Copy link
Member

hugovk commented Oct 11, 2024

@python-pillow/pillow-team What do you think?

@aclark4life
Copy link
Member

aclark4life commented Oct 11, 2024

Probably one of the few good times to make a breaking change (from 10.x to 11) and worst case you could follow up with options 1 or 2 in 11.0.1 if some unanticipated breakage occurred in the wild. 🤷

@hugovk
Copy link
Member

hugovk commented Oct 12, 2024

We've usually been cautious about breaking changes without at least a year of deprecations.

I would prefer alternative 1 or 2.

This PR is to improve the API, but motivated by adding type hints, and not user demand, so I would err on bring cautious.

@radarhere
Copy link
Member Author

Ok, I've pushed a commit to implement 2 - only changing the behaviour for lambda_eval() and unsafe_eval(), the methods that were added in 10.3.0.

@radarhere radarhere changed the title Return boolean from ImageMath comparison operations Return boolean from ImageMath lambda_eval and unsafe_eval comparison operations Oct 12, 2024
@radarhere
Copy link
Member Author

I've created #8464 to implement 1 - I've realised that the current form does have conceivably useful functionality. For example, you can invert the upper left quadrant of hopper.

>>> from PIL import Image, ImageMath
>>> im = Image.open("Tests/images/hopper.png").convert("1")
>>> b = Image.new("1", (128, 128))
>>> for x in range(64):
...   for y in range(64):
...     b.putpixel((x, y), 1)
... 
>>> ImageMath.eval("(A != B) * 65535", A=im, B=b).save("out.png")

out

@radarhere radarhere closed this Oct 12, 2024
@radarhere radarhere deleted the imagemath branch October 13, 2024 12:19
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 this pull request may close these issues.

4 participants