Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

Image.Image() == Image.Image() fails with AttributeError

Open spiiph opened this issue 2 years ago • 11 comments

What did you do?

I'm using the object constructor Image.Image() to create dummy Image objects for testing. I then compare two objects constructed in this way: Image.Image() == Image.Image()

What did you expect to happen?

I expect the comparison of two objects constructed in this way to return True. (I am aware that the recommendation is to use factory functions to create Image objects. However, would still expect the above to work.)

What actually happened?

The comparison fails with AttributeError:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/[...]/lib64/python3.8/site-packages/PIL/Image.py", line 605, in __eq__
    and self.getpalette() == other.getpalette()
  File "/[...]/lib64/python3.8/site-packages/PIL/Image.py", line 1522, in getpalette
    mode = self.im.getpalettemode()
AttributeError: 'NoneType' object has no attribute 'getpalettemode'

What are your OS, Python and Pillow versions?

  • OS: RHEL8
  • Python: 3.8
  • Pillow: 9.5.0, 10.0.1
from PIL import Image
Image.Image() == Image.Image()

spiiph avatar Oct 11 '23 10:10 spiiph

If you'll bear with me - is this the only operation that you expect to work on an Image()? Or do you think other operations should work as well?

radarhere avatar Oct 11 '23 11:10 radarhere

This was the only use case I had so I haven't really looked. I'm sure there are some where one would expect a None-result rather than an exception, e.g for Image.mode, Image.size, etc. But that might be a fairly large redesign. An option could of course be to disallow direct use of the constructor, something like

    def __call__(cls, *args, **kwargs):
        raise TypeError(
            f"{cls.__module__}.{cls.__qualname__} has no public constructor"
        )

    def _create(cls: Type[T], *args: Any, **kwargs: Any) -> T:
        return super().__call__(*args, **kwargs)  # type: ignore

spiiph avatar Oct 11 '23 11:10 spiiph

I've actually found a simple solution that should work. See what you think of #7457

radarhere avatar Oct 11 '23 12:10 radarhere

Looks like a reasonable fix. Thanks!

spiiph avatar Oct 11 '23 13:10 spiiph

I'm wondering if there's a better motivation for adding (and supporting "forever") a new Image.Image() API, against the current recommendation, beyond wanting to create dummy images for testing and being able to compare them?

Can you achieve what you want with existing APIs?

hugovk avatar Oct 11 '23 22:10 hugovk

I thought about that as well, that's why I hinted at perhaps "hiding" or "blocking" the Image() constructor. I believe (but am not 100% certain and have not tested) that this worked with maybe 9.3.0?

I am right now working around the problem by using Image.new(), so it is indeed possible to achieve what I want.

spiiph avatar Oct 12 '23 08:10 spiiph

It's the same with 9.3.0 and 10.0.1.

Thanks, I suggest you use Image.new() instead.

9.3.0

Python 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from PIL import Image
>>> Image.__version__
'9.3.0'
>>> Image.Image() == Image.Image()
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1                                                                                    │
│                                                                                                  │
│ /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/PIL/Image.py:632 │
│ in __eq__                                                                                        │
│                                                                                                  │
│    629 │   │   │   and self.size == other.size                                                   │
│    630 │   │   │   and self.info == other.info                                                   │
│    631 │   │   │   and self._category == other._category                                         │
│ ❱  632 │   │   │   and self.getpalette() == other.getpalette()                                   │
│    633 │   │   │   and self.tobytes() == other.tobytes()                                         │
│    634 │   │   )                                                                                 │
│    635                                                                                           │
│                                                                                                  │
│ /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/PIL/Image.py:147 │
│ 5 in getpalette                                                                                  │
│                                                                                                  │
│   1472 │   │                                                                                     │
│   1473 │   │   self.load()                                                                       │
│   1474 │   │   try:                                                                              │
│ ❱ 1475 │   │   │   mode = self.im.getpalettemode()                                               │
│   1476 │   │   except ValueError:                                                                │
│   1477 │   │   │   return None  # no palette                                                     │
│   1478 │   │   if rawmode is None:                                                               │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'NoneType' object has no attribute 'getpalettemode'

10.0.1

Python 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(.startup.py)
(imported collections, datetime as dt, dis, itertools, json, math, os, pprint, re, sys, time)
>>> from PIL import Image
>>> Image.Image()
<PIL.Image.Image image mode= size=0x0 at 0x10082B0B0>
>>> Image.__version__
'10.0.1'
>>> Image.Image() == Image.Image()
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1                                                                                    │
│                                                                                                  │
│ /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/PIL/Image.py:605 │
│ in __eq__                                                                                        │
│                                                                                                  │
│    602 │   │   │   and self.mode == other.mode                                                   │
│    603 │   │   │   and self.size == other.size                                                   │
│    604 │   │   │   and self.info == other.info                                                   │
│ ❱  605 │   │   │   and self.getpalette() == other.getpalette()                                   │
│    606 │   │   │   and self.tobytes() == other.tobytes()                                         │
│    607 │   │   )                                                                                 │
│    608                                                                                           │
│                                                                                                  │
│ /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/PIL/Image.py:152 │
│ 2 in getpalette                                                                                  │
│                                                                                                  │
│   1519 │   │                                                                                     │
│   1520 │   │   self.load()                                                                       │
│   1521 │   │   try:                                                                              │
│ ❱ 1522 │   │   │   mode = self.im.getpalettemode()                                               │
│   1523 │   │   except ValueError:                                                                │
│   1524 │   │   │   return None  # no palette                                                     │
│   1525 │   │   if rawmode is None:                                                               │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'NoneType' object has no attribute 'getpalettemode'

hugovk avatar Oct 12 '23 11:10 hugovk

Then I misremembered. Thanks for checking. I will use Image.new(), but will still consider it odd that an object created with the default constructor is not comparable. :)

spiiph avatar Oct 12 '23 11:10 spiiph

It's not just not comparable, it in totally wrong state. I believe we should prevent from creating objects like this, but this requires further discussion.

homm avatar Oct 12 '23 12:10 homm

Yes, that would make the most sense.

spiiph avatar Oct 12 '23 13:10 spiiph

I've created #7461 as a suggestion instead, raising a TypeError.

radarhere avatar Oct 13 '23 08:10 radarhere