From 318536c09e05e2d8a67bee0bba74a3145311c116 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Tue, 15 Aug 2023 16:18:52 -0700 Subject: [PATCH] Streamline Python wheel workflow, and add tests and a CMake setup for bindings (#1517) - Remove the libdeflate patch, not actually necessary - Add a CMake build setup, controlled ``OPENEXR_BUILD_PYTHON``, off by default - Update the doc strings - Add ``#define PY_SSIZE_T_CLEAN``, apparently required in Python 3.10 for ``Py_BuildValue("s#")``. PreviewImage attributes crash otherwise. - Add a more extensive unit test - Disable building PyPy wheels, since the bindings don't work properly (the ``channels()`` method fails in ``test_unittests.py`` when built against PyPy) Signed-off-by: Cary Phillips --- .github/workflows/python-wheels.yml | 96 +++----- CMakeLists.txt | 4 + src/wrappers/python/CMakeLists.txt | 29 +++ src/wrappers/python/OpenEXR.cpp | 3 +- src/wrappers/python/setup.py | 26 +- src/wrappers/python/tests/test_unittest.py | 265 +++++++++++++++++++++ 6 files changed, 341 insertions(+), 82 deletions(-) create mode 100644 src/wrappers/python/CMakeLists.txt create mode 100644 src/wrappers/python/tests/test_unittest.py diff --git a/.github/workflows/python-wheels.yml b/.github/workflows/python-wheels.yml index 3df9cbbf3c..67ab3a92a1 100644 --- a/.github/workflows/python-wheels.yml +++ b/.github/workflows/python-wheels.yml @@ -1,8 +1,34 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) Contributors to the OpenEXR Project. +# + name: Python wheels on: push: - branches: [ $default-branch, main, python ] + branches-ignore: + - RB-2.* + tags-ignore: + - v1.* + - v2.* + paths: + - '**' + - '!**.md' + - '!website/**' + - 'website/src/**' + - '!bazel/**' + pull_request: + branches-ignore: + - RB-2.* + tags-ignore: + - v1.* + - v2.* + paths: + - '**' + - '!**.md' + - '!website/**' + - 'website/src/**' + - '!bazel/**' jobs: build_wheels: @@ -12,30 +38,13 @@ jobs: matrix: os: [ubuntu-22.04, windows-latest, macOS-latest] env: - # Skip 32-bit wheels builds - CIBW_SKIP: "*-win32 *_i686" + # Skip 32-bit wheels builds. + # Also skip the PyPy builds, since they fail the unittests + CIBW_SKIP: "*-win32 *_i686 pp*" CIBW_BEFORE_BUILD: > - echo "Installing Zlib..." && - cd zlib.build && - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib ../zlib && - cmake --build ./ --config Release --clean-first && - cmake --install ./ --config Release && - cd .. && - echo "Installing libDeflate..." && - cd libdeflate.build && - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib -DLIBDEFLATE_BUILD_SHARED_LIB=OFF -DLIBDEFLATE_USE_SHARED_LIB=OFF -DBUILD_SHARED_LIBS=OFF ../libdeflate && - cmake --build ./ --config Release --clean-first && - cmake --install ./ --config Release && - cd .. && - echo "Installing Imath-3.1.9..." && - cd imath.build && - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib -DBUILD_TESTING=OFF -DBUILD_SHARED_LIBS=OFF ../imath && - cmake --build ./ --config Release --clean-first && - cmake --install ./ --config Release && - cd .. && echo "Installing OpenEXR..." && cd openexr.build && - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib -DBUILD_TESTING=OFF -DOPENEXR_INSTALL_EXAMPLES=OFF -DBUILD_SHARED_LIBS=OFF -DOPENEXR_FORCE_INTERNAL_DEFLATE=ON ../ && + cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib -DBUILD_TESTING=OFF -DOPENEXR_INSTALL_EXAMPLES=OFF -DOPENEXR_BUILD_TOOLS=OFF -DBUILD_SHARED_LIBS=OFF -DOPENEXR_FORCE_INTERNAL_DEFLATE=ON -DOPENEXR_FORCE_INTERNAL_IMATH=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON ../ && cmake --build ./ --config Release --clean-first && cmake --install ./ --config Release && cd .. @@ -61,52 +70,9 @@ jobs: - name: Create folders run: | - mkdir -p ${{github.workspace}}/zlib.build - mkdir -p ${{github.workspace}}/libdeflate.build - mkdir -p ${{github.workspace}}/imath.build mkdir -p ${{github.workspace}}/openexr.build mkdir -p ${{github.workspace}}/openexr.install - - name: download Zlib source code - uses: suisei-cn/actions-download-file@v1.4.0 - with: - url: https://github.com/madler/zlib/releases/download/v1.2.13/zlib-1.2.13.tar.gz - target: ${{github.workspace}}/ - - - name: Extract Zlib - run: | - tar -xvzf zlib-1.2.13.tar.gz -C ${{github.workspace}}/ - mv zlib-1.2.13 zlib - rm zlib-1.2.13.tar.gz - - - name: download libDeflate source code - uses: suisei-cn/actions-download-file@v1.4.0 - with: - url: https://github.com/ebiggers/libdeflate/archive/refs/tags/v1.18.tar.gz - target: ${{github.workspace}}/ - - - name: Extract libDeflate - run: | - tar -xvzf v1.18.tar.gz -C ${{github.workspace}}/ - mv libdeflate-1.18 libdeflate - rm v1.18.tar.gz - - - name: Patch libDeflate - run: | - patch -u libdeflate/CMakeLists.txt -i ${{github.workspace}}/src/wrappers/python/libdeflate.patch - - - name: download Imath source code - uses: suisei-cn/actions-download-file@v1.4.0 - with: - url: https://github.com/AcademySoftwareFoundation/Imath/archive/refs/tags/v3.1.9.tar.gz - target: ${{github.workspace}}/ - - - name: Extract Imath - run: | - tar -xvzf v3.1.9.tar.gz -C ${{github.workspace}}/ - mv Imath-3.1.9 imath - rm v3.1.9.tar.gz - - name: Build wheels run: python -m cibuildwheel --output-dir wheelhouse diff --git a/CMakeLists.txt b/CMakeLists.txt index 4932788149..02453a4498 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -137,3 +137,7 @@ if (NOT OPENEXR_IS_SUBPROJECT) add_subdirectory(website/src) endif() +option(OPENEXR_BUILD_PYTHON "Set ON to build python bindings") +if (OPENEXR_BUILD_PYTHON AND NOT OPENEXR_IS_SUBPROJECT) + add_subdirectory(src/wrappers/python) +endif() diff --git a/src/wrappers/python/CMakeLists.txt b/src/wrappers/python/CMakeLists.txt new file mode 100644 index 0000000000..5fc50a2508 --- /dev/null +++ b/src/wrappers/python/CMakeLists.txt @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenEXR Project. + +if(NOT "${CMAKE_PROJECT_NAME}" STREQUAL "OpenEXR") + cmake_minimum_required(VERSION 3.12) + project(PyOpenEXR) + find_package(OpenEXR) +endif() + +get_cmake_property(_variableNames VARIABLES) +list (SORT _variableNames) +foreach (_variableName ${_variableNames}) + message(STATUS "${_variableName}=${${_variableName}}") +endforeach() + +add_library (PyOpenEXR SHARED OpenEXR.cpp) + +set (Python_ADDITIONAL_VERSIONS 3) +find_package (PythonLibs REQUIRED) +find_package (PythonInterp REQUIRED) + +include_directories ("${PYTHON_INCLUDE_DIRS}") + +set_target_properties (PyOpenEXR PROPERTIES PREFIX "") +set_target_properties (PyOpenEXR PROPERTIES OUTPUT_NAME "OpenEXR") +set_target_properties (PyOpenEXR PROPERTIES SUFFIX ".so") + +target_link_libraries (PyOpenEXR "${PYTHON_LIBRARIES}" OpenEXR::OpenEXR) + diff --git a/src/wrappers/python/OpenEXR.cpp b/src/wrappers/python/OpenEXR.cpp index 3ddaaf1eb4..4d10116405 100644 --- a/src/wrappers/python/OpenEXR.cpp +++ b/src/wrappers/python/OpenEXR.cpp @@ -1,8 +1,9 @@ // // SPDX-License-Identifier: BSD-3-Clause -// Copyright (c) DreamWorks Animation LLC and Contributors of the OpenEXR Project +// Copyright (c) Contributors to the OpenEXR Project. // +#define PY_SSIZE_T_CLEAN // required for Py_BuildValue("s#") for Python 3.10 #include #if PY_VERSION_HEX < 0x02050000 && !defined(PY_SSIZE_T_MIN) diff --git a/src/wrappers/python/setup.py b/src/wrappers/python/setup.py index 81dac355a4..e92ba454ba 100644 --- a/src/wrappers/python/setup.py +++ b/src/wrappers/python/setup.py @@ -1,13 +1,10 @@ -# SPDX-License-Identifier: BSD-3-Clause -# Copyright (c) Contributors to the OpenEXR Project. - from setuptools import setup, Extension import os import platform import re -DESC = """Python bindings for ILM's OpenEXR image file format. +DESC = """Python bindings for the OpenEXR image file format. This is a script to autobuild the wheels using github actions. Please, do not use it manually @@ -15,7 +12,7 @@ If you detect any problem, please feel free to report the issue on the GitHub page: -https://github.com/sanguinariojoe/pip-openexr/issues +https://github.com/AcademySoftwareFoundation/openexr/issues """ @@ -29,19 +26,16 @@ version = f"{version_major}.{version_minor}.{version_patch}" libs=[] -libs_static=['z', - f'OpenEXR-{version_major}_{version_minor}', +libs_static=[f'OpenEXR-{version_major}_{version_minor}', f'IlmThread-{version_major}_{version_minor}', f'Iex-{version_major}_{version_minor}', - f'Imath-3_1', - f'OpenEXRCore-{version_major}_{version_minor}', - f'deflate'] + f'Imath-{version_major}_{version_minor}', + f'OpenEXRCore-{version_major}_{version_minor}' + ] definitions = [('PYOPENEXR_VERSION_MAJOR', f'{version_major}'), ('PYOPENEXR_VERSION_MINOR', f'{version_minor}'), ('PYOPENEXR_VERSION_PATCH', f'{version_patch}'),] if platform.system() == "Windows": - libs_static[0] = 'zlibstatic' - libs_static[-1] = 'deflatestatic' definitions = [('PYOPENEXR_VERSION', f'\\"{version}\\"')] extra_compile_args = [] if platform.system() == 'Darwin': @@ -65,10 +59,10 @@ setup(name='OpenEXR', - author = 'James Bowman', - author_email = 'jamesb@excamera.com', - url = 'https://github.com/sanguinariojoe/pip-openexr', - description = "Python bindings for ILM's OpenEXR image file format", + author = 'Contributors to the OpenEXR Project', + author_email = 'info@openexr.com', + url = 'https://github.com/AcademySoftwareFoundation/openexr', + description = "Python bindings for the OpenEXR image file format", long_description = DESC, version=version, ext_modules=[ diff --git a/src/wrappers/python/tests/test_unittest.py b/src/wrappers/python/tests/test_unittest.py new file mode 100644 index 0000000000..5061ac2b91 --- /dev/null +++ b/src/wrappers/python/tests/test_unittest.py @@ -0,0 +1,265 @@ +#!/usr/bin/env python3 + +# +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenEXR Project. +# + +from __future__ import print_function +import sys +import os +import random +from array import array + +import Imath +import OpenEXR + +test_dir = os.path.dirname(os.path.abspath(__file__)) + +FLOAT = Imath.PixelType(Imath.PixelType.FLOAT) +UINT = Imath.PixelType(Imath.PixelType.UINT) +HALF = Imath.PixelType(Imath.PixelType.HALF) + +testList = [] + +# +# Write a simple exr file, read it back and confirm the data is the same. +# + +def test_write_read(): + + width = 100 + height = 100 + size = width * height + + h = OpenEXR.Header(width,height) + h['channels'] = {'R' : Imath.Channel(FLOAT), + 'G' : Imath.Channel(FLOAT), + 'B' : Imath.Channel(FLOAT), + 'A' : Imath.Channel(FLOAT)} + o = OpenEXR.OutputFile("write.exr", h) + r = array('f', [n for n in range(size*0,size*1)]).tobytes() + g = array('f', [n for n in range(size*1,size*2)]).tobytes() + b = array('f', [n for n in range(size*2,size*3)]).tobytes() + a = array('f', [n for n in range(size*3,size*4)]).tobytes() + channels = {'R' : r, 'G' : g, 'B' : b, 'A' : a} + o.writePixels(channels) + o.close() + + i = OpenEXR.InputFile("write.exr") + h = i.header() + assert r == i.channel('R') + assert g == i.channel('G') + assert b == i.channel('B') + assert a == i.channel('A') + + print("write_read ok") + +testList.append(("test_write_read", test_write_read)) + +def test_level_modes(): + + assert Imath.LevelMode("ONE_LEVEL").v == Imath.LevelMode(Imath.LevelMode.ONE_LEVEL).v + assert Imath.LevelMode("MIPMAP_LEVELS").v == Imath.LevelMode(Imath.LevelMode.MIPMAP_LEVELS).v + assert Imath.LevelMode("RIPMAP_LEVELS").v == Imath.LevelMode(Imath.LevelMode.RIPMAP_LEVELS).v + + print("level modes ok") + +testList.append(("test_level_modes", test_level_modes)) + +# +# Write an image as UINT, read as FLOAT, and the reverse. +# +def test_conversion(): + codemap = { 'f': FLOAT, 'I': UINT } + original = [0, 1, 33, 79218] + for frm_code,to_code in [ ('f','I'), ('I','f') ]: + hdr = OpenEXR.Header(len(original), 1) + hdr['channels'] = {'L': Imath.Channel(codemap[frm_code])} + x = OpenEXR.OutputFile("out.exr", hdr) + x.writePixels({'L': array(frm_code, original).tobytes()}) + x.close() + + xin = OpenEXR.InputFile("out.exr") + assert array(to_code, xin.channel('L', codemap[to_code])).tolist() == original + + print("conversion ok") + +testList.append(("test_conversion", test_conversion)) + +# +# Confirm failure on reading from non-exist location +# + +def test_invalid_input(): + try: + OpenEXR.InputFile("/bad/place") + except: + pass + else: + assert 0 + +testList.append(("test_invalid_input", test_invalid_input)) + +# +# Confirm failure on writing to invalid location +# + +def test_invalid_output(): + + try: + hdr = OpenEXR.Header(640, 480) + OpenEXR.OutputFile("/bad/place", hdr) + except: + pass + else: + assert 0 + + print("invalid output ok") + +testList.append(("test_invalid_output", test_invalid_output)) + +def test_one(): + oexr = OpenEXR.InputFile("write.exr") + + header = oexr.header() + + default_size = len(oexr.channel('R')) + half_size = len(oexr.channel('R', Imath.PixelType(Imath.PixelType.HALF))) + float_size = len(oexr.channel('R', Imath.PixelType(Imath.PixelType.FLOAT))) + uint_size = len(oexr.channel('R', Imath.PixelType(Imath.PixelType.UINT))) + + assert default_size in [ half_size, float_size, uint_size] + assert float_size == uint_size + assert (float_size / 2) == half_size + + assert len(oexr.channel('R', + pixel_type = FLOAT, + scanLine1 = 10, + scanLine2 = 10)) == (4 * (header['dataWindow'].max.x + 1)) + + + data = b" " * (4 * 100 * 100) + h = OpenEXR.Header(100,100) + x = OpenEXR.OutputFile("out.exr", h) + x.writePixels({'R': data, 'G': data, 'B': data}) + x.close() + + print("one ok") + +testList.append(("test_one", test_one)) + +# +# Check that the channel method and channels method return the same data +# + +def test_channel_channels(): + + aexr = OpenEXR.InputFile("write.exr") + acl = sorted(aexr.header()['channels'].keys()) + a = [aexr.channel(c) for c in acl] + b = aexr.channels(acl) + + assert a == b + + print("channels ok") + +testList.append(("test_channel_channels", test_channel_channels)) + +def test_types(): + for original in [ [0,0,0], list(range(10)), list(range(100,200,3)) ]: + for code,t in [ ('I', UINT), ('f', FLOAT) ]: + data = array(code, original).tobytes() + hdr = OpenEXR.Header(len(original), 1) + hdr['channels'] = {'L': Imath.Channel(t)} + + x = OpenEXR.OutputFile("out.exr", hdr) + x.writePixels({'L': data}) + x.close() + + xin = OpenEXR.InputFile("out.exr") + # Implicit type + assert array(code, xin.channel('L')).tolist() == original + # Explicit typen + assert array(code, xin.channel('L', t)).tolist() == original + # Explicit type as kwarg + assert array(code, xin.channel('L', pixel_type = t)).tolist() == original + + print("types ok") + +testList.append(("test_types", test_types)) + +def test_invalid_pixeltype(): + oexr = OpenEXR.InputFile("write.exr") + FLOAT = Imath.PixelType.FLOAT + try: + f.channel('R',FLOAT) + except: + pass + else: + assert 0 + + print("invalid pixeltype ok") + +testList.append(("test_invalid_pixeltype", test_invalid_pixeltype)) + +# +# Write arbitrarily named channels. +# + +def test_write_mchannels(): + hdr = OpenEXR.Header(100, 100) + for chans in [ set("a"), set(['foo', 'bar']), set("abcdefghijklmnopqstuvwxyz") ]: + hdr['channels'] = dict([(nm, Imath.Channel(Imath.PixelType(Imath.PixelType.FLOAT))) for nm in chans]) + x = OpenEXR.OutputFile("out0.exr", hdr) + data = array('f', [0] * (100 * 100)).tobytes() + x.writePixels(dict([(nm, data) for nm in chans])) + x.close() + assert set(OpenEXR.InputFile('out0.exr').header()['channels']) == chans + + print("mchannels ok") + +testList.append(("test_write_mchannels", test_write_mchannels)) + +def load_red(filename): + oexr = OpenEXR.InputFile(filename) + return oexr.channel('R') + +# +# Write the pixels to two images, first as a single call, +# then as multiple calls. Verify that the images are identical. +# + +def test_write_chunk(): + for w,h,step in [(100, 10, 1), (64,48,6), (1, 100, 2), (640, 480, 4)]: + data = array('f', [ random.random() for x in range(w * h) ]).tobytes() + + hdr = OpenEXR.Header(w,h) + x = OpenEXR.OutputFile("out0.exr", hdr) + x.writePixels({'R': data, 'G': data, 'B': data}) + x.close() + + hdr = OpenEXR.Header(w,h) + x = OpenEXR.OutputFile("out1.exr", hdr) + for y in range(0, h, step): + subdata = data[y * w * 4:(y+step) * w * 4] + x.writePixels({'R': subdata, 'G': subdata, 'B': subdata}, step) + x.close() + + oexr0 = load_red("out0.exr") + oexr1 = load_red("out1.exr") + assert oexr0 == oexr1 + + print("chunk ok") + +testList.append(("test_write_chunk", test_write_chunk)) + +for test in testList: + funcName = test[0] + print ("") + print ("Running {}".format (funcName)) + test[1]() + +print() +print("all ok") +