__eq__ works unexpectedly because the readonly tag changed oddly

[NewUserHa]

What did you do?

try to compare img, like img != last_img

What did you expect to happen?

img != last_img should be False when didn't change the clipboard content.

What actually happened?

img != last_img is True

What are your OS, Python and Pillow versions?

• OS: win10 1803
• Python: 3.8.10
• Pillow: 7.0.0

while 1:
    img = ImageGrab.grabclipboard()
    if img and img != last_img:
        break
    time.sleep(0.5)
if ...:
    last_img = None
else:
    last_img = img

Addition:

found the reason is

Pillow/src/PIL/Image.py

Line 630 in 05b6336

and self.readonly == other.readonly

it will change to 0 after the self.tobytes()(2 lines after it) is executed, and cause a wrong == answer even the picture in the clipboard hasn't been touched at all.

The fix probably is moving that line to the last?
is the behavior intended?

[NewUserHa]

from PIL import Image
img = Image.new('RGB', (1, 1))
img.readonly # 0
img.tobytes()
img.readonly # 0

img = ImageGrab.grabclipboard()
img.readonly # 1
img.tobytes()
img.readonly # 0

need a PR?

[radarhere]

Some notes.

• ImageGrab.grabclipboard() has readonly of 1 on Windows yes, but not on macOS.
• readonly becomes 0 after load(), not just tobytes()

The readonly condition was added in #638. Since it was his PR, perhaps @hugovk has thoughts on this?

[NewUserHa]

Why would .tobytes() cause .readonly change?
Is .readonly a useful public property or a private property? And if users use both of them together, will it cause this issue to be reported again?

[radarhere]

It is not tobytes() directly that causes the change. Rather, tobytes() calls load() which causes the change.

In my understanding, readonly is a flag that describes if an image is memory-mapped. For such images, we don't want to change the underlying data, and will have to copy it before making modifications.

readonly is initially set to 1 just in case the image is actually readonly.

Pillow/src/PIL/ImageFile.py

Line 93 in 3f77466

self.readonly = 1 # until we know better

Once the image is loaded, then the true situation is determined. In this case, the image is not readonly.

readonly is not considered private, since it doesn't begin with _. I suppose it could have been made private when it was created, since I can't think of a way that it is helpful to users.

#5930 means that when Pillow 9.1 is released, due out on April 1, readonly will not be considered when checking image equality, so this issue should be resolved.

[NewUserHa]

thank you for the detail reason.
since .readonly is a flag, it should be precise. maybe should set it within the function ImageGrab.grabclipboard() to 0 to resolve the real issue?
or mark the flag private and tell users not to use it since it's not always precise?

[radarhere]

I don't think there is anything specific in ImageGrab.grabclipboard() that relates to readonly. If there is a problem, it is more general than that.

If we were to change it, I would be more interested in setting it to None initially, so that there is a distinction made.

Pillow values backwards compatibility, and I'm not personally convinced that readonly is problematic enough to warrant a change - you yourself are not even directly using it in your code. However, if you feel this is worthwhile, feel free to create a PR.

[NewUserHa]

the "setting it to None initially" looks like a better idea.