From ae187cacb2a18323e3a0d9bfe5f8739a25d5153b Mon Sep 17 00:00:00 2001 From: wiredfool Date: Sat, 29 Mar 2025 23:18:48 +0000 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- .github/workflows/test-windows.yml | 13 ++---- Tests/test_arrow.py | 4 +- docs/reference/arrow_support.rst | 63 +++++++++++++++--------------- src/PIL/Image.py | 21 ++++------ src/_imaging.c | 6 +-- 5 files changed, 46 insertions(+), 61 deletions(-) diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml index e248eafc0..0b45f93c9 100644 --- a/.github/workflows/test-windows.yml +++ b/.github/workflows/test-windows.yml @@ -35,17 +35,11 @@ jobs: strategy: fail-fast: false matrix: - python-version: [ "3.10", "3.11", "3.12", "3.13" ] + python-version: ["pypy3.11", "pypy3.10", "3.10", "3.11", "3.12", "3.13", "3.14"] architecture: ["x64"] os: ["windows-latest"] - pyarrow: ["true"] include: # Test the oldest Python on 32-bit - - { python-version: "3.9", architecture: "x86", os: "windows-2019", pyarrow: "false" } - # test the non-pyarrow capable ones - - { python-version: "3.14", architecture: "x64", os: "windows-latest", pyarrow: "false" } - - { python-version: "pypy3.11", architecture: "x64", os: "windows-latest", pyarrow: "false" } - - { python-version: "pypy3.10", architecture: "x64", os: "windows-latest", pyarrow: "false" } timeout-minutes: 30 name: Python ${{ matrix.python-version }} (${{ matrix.architecture }}) @@ -92,10 +86,9 @@ jobs: run: | python3 -m pip install PyQt6 - - name: Install PyArrow Test Dependency - if: "matrix.pyarrow == 'true'" + - name: Install PyArrow dependency run: | - python3 -m pip install --only-binary=:all: pyarrow + python3 -m pip install --only-binary=:all: pyarrow || true - name: Install dependencies id: install diff --git a/Tests/test_arrow.py b/Tests/test_arrow.py index 3338b6c6a..a86757216 100644 --- a/Tests/test_arrow.py +++ b/Tests/test_arrow.py @@ -4,9 +4,7 @@ import pytest from PIL import Image -from .helper import ( - hopper, -) +from .helper import hopper @pytest.mark.parametrize( diff --git a/docs/reference/arrow_support.rst b/docs/reference/arrow_support.rst index c9f12c64b..9f14f1407 100644 --- a/docs/reference/arrow_support.rst +++ b/docs/reference/arrow_support.rst @@ -4,71 +4,72 @@ Arrow Support ============= -Arrow is an in memory data exchange format that is the spritual -successor to the numpy array interface. It provides for zero copy -access to columnar data, which in our case is Image data. +`Arrow `__ +is an in-memory data exchange format that is the spiritual +successor to the NumPy array interface. It provides for zero-copy +access to columnar data, which in our case is ``Image`` data. -The goal with Arrow is to provide native zero-copy interop with any -arrow provider or consumer in the Python ecosystem. +The goal with Arrow is to provide native zero-copy interoperability +with any Arrow provider or consumer in the Python ecosystem. -.. warning:: Zero-copy does not mean zero allocation -- The internal +.. warning:: Zero-copy does not mean zero allocation -- the internal memory layout of Pillow images contains an allocation for row pointers, so there is a non-zero, but significantly smaller than a - full copy memory cost to reading an arrow image. + full-copy memory cost to reading an Arrow image. Data Formats ============ -Pillow currently supports exporting arrow images in all modes +Pillow currently supports exporting Arrow images in all modes **except** for ``BGR;15``, ``BGR;16`` and ``BGR;24``. This is due to -line length packing in these modes making for non-continuous memory. +line-length packing in these modes making for non-continuous memory. -For single band images, the exported array is width*height elements, -with each pixel corresponding to the appropriate arrow type. +For single-band images, the exported array is width*height elements, +with each pixel corresponding to the appropriate Arrow type. -For multiband images, the exported array is width*height fixed length -4 element arrays of uint8. This is memory compatible with the raw -image storage of 4 bytes per pixel. +For multiband images, the exported array is width*height fixed-length +four-element arrays of uint8. This is memory compatible with the raw +image storage of four bytes per pixel. -Mode ``1`` images are exported as 1 uint8 byte/pixel, as this is +Mode ``1`` images are exported as one uint8 byte/pixel, as this is consistent with the internal storage. Pillow will accept, but not produce, one other format. For any -multichannel image with 32 bit storage per pixel, Pillow will accept +multichannel image with 32-bit storage per pixel, Pillow will accept an array of width*height int32 elements, which will then be -interpreted using the mode specific interpretation of the bytes. +interpreted using the mode-specific interpretation of the bytes. -The image mode must match the arrow band format when reading single -channel images +The image mode must match the Arrow band format when reading single +channel images. Memory Allocator ================ Pillow's default memory allocator, the :ref:`block_allocator`, -allocates up to a 16MB block for images by default. Larger images +allocates up to a 16 MB block for images by default. Larger images overflow into additional blocks. Arrow requires a single continuous memory allocation, so images allocated in multiple blocks cannot be -exported in the arrow format. +exported in the Arrow format. To enable the single block allocator:: from PIL import Image Image.core.set_use_block_allocator(1) -Note that this is a global setting, not a per image setting. +Note that this is a global setting, not a per-image setting. Unsupported Features ==================== -* Table/Dataframe protocol. We currently support a single array. +* Table/dataframe protocol. We support a single array. * Null markers, producing or consuming. Null values are inferred from the mode. e.g. RGB images are stored in the first three bytes of - each 32 bit pixel, and the last byte is an implied null. -* Schema Negotiation. There is an optional schema for the requested - datatype in the arrow source interface. We currently ignore that + each 32-bit pixel, and the last byte is an implied null. +* Schema negotiation. There is an optional schema for the requested + datatype in the Arrow source interface. We ignore that parameter. -* Array Metadata. +* Array metadata. Internal Details ================ @@ -76,12 +77,12 @@ Internal Details Python Arrow C interface: https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html -The memory that is exported from the arrow interface is shared -- not +The memory that is exported from the Arrow interface is shared -- not copied, so the lifetime of the memory allocation is no longer strictly -tied to the life of the python object. +tied to the life of the Python object. The core imaging struct now has a refcount associated with it, and the -lifetime of the core image struct is now divorced from the python +lifetime of the core image struct is now divorced from the Python image object. Creating an arrow reference to the image increments the refcount, and the imaging struct is only released when the refcount -reaches 0. +reaches zero. diff --git a/src/PIL/Image.py b/src/PIL/Image.py index aa18f90d9..0dbb24d91 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -3210,16 +3210,9 @@ class SupportsArrowArrayInterface(Protocol): data interface. """ - # Sorry, no types for you until pre-commit.ci stops changing stringy - # PyCapsule type to an unimportable value type, which then fails lint. - # PyCapsules are not importable, and only available in the C-API layer. - # def __arrow_c_array__( - # self, requested_schema: 'PyCapsule' = None - # ) -> tuple['PyCapsule', 'PyCapsule']: - # raise NotImplementedError() - - # old not typed definition. - def __arrow_c_array__(self, requested_schema=None): + def __arrow_c_array__( + self, requested_schema: "PyCapsule" = None # type: ignore[name-defined] # noqa: F821, UP037 + ) -> tuple["PyCapsule", "PyCapsule"]: # type: ignore[name-defined] # noqa: F821, UP037 raise NotImplementedError() @@ -3312,7 +3305,7 @@ def fromarray(obj: SupportsArrayInterface, mode: str | None = None) -> Image: def fromarrow(obj: SupportsArrowArrayInterface, mode, size) -> Image: - """Creates an image with zero copy shared memory from an object exporting + """Creates an image with zero-copy shared memory from an object exporting the arrow_c_array interface protocol:: from PIL import Image @@ -3323,7 +3316,7 @@ def fromarrow(obj: SupportsArrowArrayInterface, mode, size) -> Image: If the data representation of the ``obj`` is not compatible with Pillow internal storage, a ValueError is raised. - Pillow images can also be converted to arrow objects:: + Pillow images can also be converted to Arrow objects:: from PIL import Image import pyarrow as pa @@ -3339,13 +3332,13 @@ def fromarrow(obj: SupportsArrowArrayInterface, mode, size) -> Image: :param size: Image size. This must match the storage of the arrow object. :returns: An Image Object - Note that according to the arrow spec, both the producer and the + Note that according to the Arrow spec, both the producer and the consumer should consider the exported array to be immutable, as unsynchronized updates will potentially cause inconsistent data. See: :ref:`arrow-support` for more detailed information - .. versionadded:: 11.2 + .. versionadded:: 11.2.0 """ if not hasattr(obj, "__arrow_c_array__"): diff --git a/src/_imaging.c b/src/_imaging.c index 24ce901bc..9eee3c82c 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -240,14 +240,14 @@ ArrowError(int err) { return ImagingError_MemoryError(); } if (err == IMAGING_ARROW_INCOMPATIBLE_MODE) { - return ImagingError_ValueError("Incompatible Pillow mode for Arrow Array"); + return ImagingError_ValueError("Incompatible Pillow mode for Arrow array"); } if (err == IMAGING_ARROW_MEMORY_LAYOUT) { return ImagingError_ValueError( "Image is in multiple array blocks, use imaging_new_block for zero copy" ); } - return ImagingError_ValueError("Unknown Error"); + return ImagingError_ValueError("Unknown error"); } void @@ -313,7 +313,7 @@ _new_arrow(PyObject *self, PyObject *args) { PyImagingNew(ImagingNewArrow(mode, xsize, ysize, schema_capsule, array_capsule) ); if (!ret) { - return ImagingError_ValueError("Invalid arrow array mode or size mismatch"); + return ImagingError_ValueError("Invalid Arrow array mode or size mismatch"); } return ret; }