Passing a numpy.ndarray as the size to Image.resize() works in 10.3.0, but not in 10.4.0

musicinmybrain commented 5 days ago

What did you do?

I encountered some new test failures in https://github.com/mikedh/trimesh/issues/2247 because passing a numpy.ndarray as the size to Image.resize() works in 10.3.0, but not in 10.4.0.

The question is whether this is a regression in Pillow, or whether passing a numpy.ndarray to Image.resize() was always “wrong,” since https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.resize calls for a “tuple,” and it only happened to work.

What did you expect to happen?

With numpy==2.0.0, pillow==10.3.0:

>>> from PIL import Image
>>> from numpy import array
>>> x = Image.new('RGB', (2, 4))
>>> x.resize(array([3, 7]))
<PIL.Image.Image image mode=RGB size=3x7 at 0x7FB58A5866C0>

What actually happened?

With numpy==2.0.0, pillow==10.4.0:

>>> from PIL import Image
>>> from numpy import array
>>> x = Image.new('RGB', (2, 4))
>>> x.resize(array([3, 7]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ben/fedora/neuro-sig/python-trimesh/_e/lib64/python3.12/site-packages/PIL/Image.py", line 2297, in resize
    if self.size == size and box == (0, 0) + self.size:
       ^^^^^^^^^^^^^^^^^
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

What are your OS, Python and Pillow versions?

OS: Fedora 40
Python: 3.12.3
Pillow: 10.4.0 (vs. 10.3.0)

While I originally encountered this building system RPM packages, this report is based on the above reproducer in a clean virtualenv using packages installed from PyPI.

--------------------------------------------------------------------
Pillow 10.4.0
Python 3.12.3 (main, Apr 17 2024, 00:00:00) [GCC 14.0.1 20240411 (Red Hat 14.0.1-0)]
--------------------------------------------------------------------
Python executable is /home/ben/fedora/neuro-sig/python-trimesh/_e/bin/python
Environment Python files loaded from /home/ben/fedora/neuro-sig/python-trimesh/_e
System Python files loaded from /usr
--------------------------------------------------------------------
Python Pillow modules loaded from /home/ben/fedora/neuro-sig/python-trimesh/_e/lib64/python3.12/site-packages/PIL
Binary Pillow modules loaded from /home/ben/fedora/neuro-sig/python-trimesh/_e/lib64/python3.12/site-packages/PIL
--------------------------------------------------------------------
--- PIL CORE support ok, compiled for 10.4.0
--- TKINTER support ok, loaded 8.6
--- FREETYPE2 support ok, loaded 2.13.2
--- LITTLECMS2 support ok, loaded 2.16
--- WEBP support ok, loaded 1.4.0
--- WEBP Transparency support ok
--- WEBPMUX support ok
--- WEBP Animation support ok
--- JPEG support ok, compiled for libjpeg-turbo 3.0.3
--- OPENJPEG (JPEG2000) support ok, loaded 2.5.2
--- ZLIB (PNG/ZIP) support ok, loaded 1.3.0.zlib-ng
--- LIBTIFF support ok, loaded 4.6.0
--- RAQM (Bidirectional Text) support ok, loaded 0.10.1, fribidi 1.0.14, harfbuzz 8.5.0
*** LIBIMAGEQUANT (Quantization method) support not installed
--- XCB (X protocol) support ok
--------------------------------------------------------------------

radarhere commented 3 days ago

The casting that allowed this to work was originally added in https://github.com/python-pillow/Pillow/pull/945 for the sake of a SciPy regression.

We stopped testing for it in https://github.com/python-pillow/Pillow/pull/3087, but left the casting.

The casting was then removed in https://github.com/python-pillow/Pillow/pull/8046

radarhere commented 3 days ago

Is there any particular reason that it is helpful for resize() to accept a NumPy array? I have to imagine that most of Pillow doesn't work well with NumPy arrays as sizes.

We're currently working on improving type hints. If you're aware of a generic type hint that supports tuples and NumPy arrays, let me know.

musicinmybrain commented 3 days ago

Is there any particular reason that it is helpful for resize() to accept a NumPy array? I have to imagine that most of Pillow doesn't work well with NumPy arrays as sizes.

In the abstract, I would say no. In practice, it’s a case of Hyrum’s Law: since the interface has historically accepted a NumPy array, some software depends on this behavior and breaks when it stops working. Suggesting a patch for trimesh was fairly straightforward, but there might be cases where it’s not so easy.

We're currently working on improving type hints. If you're aware of a generic type hint that supports tuples and NumPy arrays, let me know.

There’s collections.abc.Sequence, but a Sequence[int] may have arbitrary length. For that reason, tuple[int, int] is probably a better choice. Even if the annotation allowed non-tuple sequences, it would be necessary to ensure the implementation actually supported arbitrary sequence types.

musicinmybrain commented 3 days ago

Note that passing a list, e.g. newsize = [3, 7]; x.resize(newsize), does still work, but code wishing to respect the type annotations will need to convert to a tuple, x.resize(tuple(newsize)). That’s probably reasonable.

python-pillow / Pillow