From c9c8ab7023f645cfcab39448a8a98f8760ba378f Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Fri, 15 Dec 2023 15:14:16 -0500 Subject: [PATCH 1/8] Minimal patch for Windows build --- .gitignore | 4 +++ pyrtools/pyramids/c/convolve.h | 39 +++++++++++++------------- pyrtools/pyramids/c/internal_pointOp.c | 2 ++ pyrtools/pyramids/c/internal_pointOp.h | 8 ++++-- pyrtools/pyramids/c/meta.h | 8 ++++++ pyrtools/pyramids/c/py.c | 8 ++++++ pyrtools/pyramids/c/wrapper.py | 4 ++- setup.py | 6 ++-- 8 files changed, 54 insertions(+), 25 deletions(-) create mode 100644 pyrtools/pyramids/c/meta.h create mode 100644 pyrtools/pyramids/c/py.c diff --git a/.gitignore b/.gitignore index 18639e0..011c691 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,10 @@ __pycache__/ # C extensions *.so +# Windows +*.exp +*.obj +*.lib # Distribution / packaging .Python diff --git a/pyrtools/pyramids/c/convolve.h b/pyrtools/pyramids/c/convolve.h index 48d55f7..9f15987 100755 --- a/pyrtools/pyramids/c/convolve.h +++ b/pyrtools/pyramids/c/convolve.h @@ -13,6 +13,7 @@ #include #include +#include "meta.h" #define ABS(x) (((x)>=0) ? (x) : (-(x))) #define ROOT2 1.4142135623730951 @@ -34,22 +35,22 @@ typedef struct typedef double image_type; fptr edge_function(char *edges); -int internal_reduce(image_type *image, int x_idim, int y_idim, - image_type *filt, image_type *temp, int x_fdim, int y_fdim, - int x_start, int x_step, int x_stop, - int y_start, int y_step, int y_stop, - image_type *result, char *edges); -int internal_expand(image_type *image, - image_type *filt, image_type *temp, int x_fdim, int y_fdim, - int x_start, int x_step, int x_stop, - int y_start, int y_step, int y_stop, - image_type *result, int x_rdim, int y_rdim, char *edges); -int internal_wrap_reduce(image_type *image, int x_idim, int y_idim, - image_type *filt, int x_fdim, int y_fdim, - int x_start, int x_step, int x_stop, - int y_start, int y_step, int y_stop, - image_type *result); -int internal_wrap_expand(image_type *image, image_type *filt, int x_fdim, int y_fdim, - int x_start, int x_step, int x_stop, - int y_start, int y_step, int y_stop, - image_type *result, int x_rdim, int y_rdim); +PYRTOOLS_EXPORT int internal_reduce(image_type *image, int x_idim, int y_idim, + image_type *filt, image_type *temp, int x_fdim, int y_fdim, + int x_start, int x_step, int x_stop, + int y_start, int y_step, int y_stop, + image_type *result, char *edges); +PYRTOOLS_EXPORT int internal_expand(image_type *image, + image_type *filt, image_type *temp, int x_fdim, int y_fdim, + int x_start, int x_step, int x_stop, + int y_start, int y_step, int y_stop, + image_type *result, int x_rdim, int y_rdim, char *edges); +PYRTOOLS_EXPORT int internal_wrap_reduce(image_type *image, int x_idim, int y_idim, + image_type *filt, int x_fdim, int y_fdim, + int x_start, int x_step, int x_stop, + int y_start, int y_step, int y_stop, + image_type *result); +PYRTOOLS_EXPORT int internal_wrap_expand(image_type *image, image_type *filt, int x_fdim, int y_fdim, + int x_start, int x_step, int x_stop, + int y_start, int y_step, int y_stop, + image_type *result, int x_rdim, int y_rdim); diff --git a/pyrtools/pyramids/c/internal_pointOp.c b/pyrtools/pyramids/c/internal_pointOp.c index 3b146d5..314fd81 100644 --- a/pyrtools/pyramids/c/internal_pointOp.c +++ b/pyrtools/pyramids/c/internal_pointOp.c @@ -1,5 +1,7 @@ #include #include +#include "internal_pointOp.h" + /* Use linear interpolation on a lookup table. Taken from OBVIUS. EPS, Spring, 1987. */ diff --git a/pyrtools/pyramids/c/internal_pointOp.h b/pyrtools/pyramids/c/internal_pointOp.h index 85624c1..5c7961e 100644 --- a/pyrtools/pyramids/c/internal_pointOp.h +++ b/pyrtools/pyramids/c/internal_pointOp.h @@ -1,3 +1,5 @@ -void internal_pointop(double *im, double *res, int size, double *lut, - int lutsize, double origin, double increment, - int warnings); +#include "meta.h" + +PYRTOOLS_EXPORT void internal_pointop(double *im, double *res, int size, double *lut, + int lutsize, double origin, double increment, + int warnings); diff --git a/pyrtools/pyramids/c/meta.h b/pyrtools/pyramids/c/meta.h new file mode 100644 index 0000000..d3ccae5 --- /dev/null +++ b/pyrtools/pyramids/c/meta.h @@ -0,0 +1,8 @@ +#pragma once + +// this macro controls symbol visibility in the shared library +#if defined _WIN32 || defined __MINGW32__ +#define PYRTOOLS_EXPORT __declspec(dllexport) +#else +#define PYRTOOLS_EXPORT +#endif diff --git a/pyrtools/pyramids/c/py.c b/pyrtools/pyramids/c/py.c new file mode 100644 index 0000000..e0550b1 --- /dev/null +++ b/pyrtools/pyramids/c/py.c @@ -0,0 +1,8 @@ + +// This is required to allow the use of the python setuptools Extension +// class on Windows. Alternatives would be building ourselves or using +// a tool such as Meson which doesn't assume you're using the Python +// C API directly. +void* PyInit_wrapConv(void) { + return 0; +} diff --git a/pyrtools/pyramids/c/wrapper.py b/pyrtools/pyramids/c/wrapper.py index bb88bf3..ea744c8 100644 --- a/pyrtools/pyramids/c/wrapper.py +++ b/pyrtools/pyramids/c/wrapper.py @@ -9,7 +9,9 @@ # the wrapConv.so file can have some system information after it from the compiler, so we just find # whatever it is called -libpath = glob.glob(os.path.join(os.path.dirname(os.path.realpath(__file__)), 'wrapConv*.so')) +lib_folder = os.path.dirname(os.path.realpath(__file__)) +libpath = glob.glob(os.path.join(lib_folder, 'wrapConv*.so')) + \ + glob.glob(os.path.join(lib_folder, 'wrapConv.*.pyd')) # print(libpath) # load the c library diff --git a/setup.py b/setup.py index a89bf70..5381061 100644 --- a/setup.py +++ b/setup.py @@ -31,11 +31,13 @@ 'tqdm>=4.29', 'requests>=2.21'], ext_modules=[Extension('pyrtools.pyramids.c.wrapConv', - sources=['pyrtools/pyramids/c/convolve.c', + sources=['pyrtools/pyramids/c/py.c', + 'pyrtools/pyramids/c/convolve.c', 'pyrtools/pyramids/c/edges.c', 'pyrtools/pyramids/c/wrap.c', 'pyrtools/pyramids/c/internal_pointOp.c'], - depends=['pyrtools/pyramids/c/convolve.h', + depends=['pyrtools/pyramids/c/meta.h', + 'pyrtools/pyramids/c/convolve.h', 'pyrtools/pyramids/c/internal_pointOp.h'], extra_compile_args=['-fPIC', '-shared'])], tests='TESTS', From 5be47bdc5f00db9578f412a8e5de4d9456f6f4d6 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Fri, 15 Dec 2023 15:14:44 -0500 Subject: [PATCH 2/8] CI: run on Windows, macOS --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 615b3ef..6030dd4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,9 +49,10 @@ jobs: - name: Run notebooks run: jupyter execute ${{ matrix.notebook }}.ipynb --kernel_name=python3 tests: - runs-on: ubuntu-latest + runs-on: ${{matrix.os}} strategy: matrix: + os: [ubuntu-latest, macos-latest, windows-latest] python-version: [3.7, 3.8, 3.9, '3.10'] fail-fast: false name: Run tests From d80c62055780a9788163ddc6d5b5fa377a25ca31 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Fri, 15 Dec 2023 15:22:14 -0500 Subject: [PATCH 3/8] Build version-independent wheels, update deploy action to build --- .github/workflows/deploy.yml | 57 ++++++++++++++++++++++++++++++------ setup.py | 13 ++++++++ 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index e71113a..8fa19f4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -4,20 +4,59 @@ on: types: [published] jobs: + build-wheels: + name: Make ${{ matrix.os }} wheels + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: ["macos-latest", "ubuntu-latest", "windows-latest"] + fail-fast: false + + steps: + - uses: actions/checkout@v3 + + - name: "Build wheels" + uses: pypa/cibuildwheel@v2.8.1 + env: + CIBW_MANYLINUX_X86_64_IMAGE: manylinux2014 + CIBW_BUILD: cp37-* cp38-* cp39-* cp310-* + CIBW_SKIP: "*musllinux*" + CIBW_ARCHS: native + CIBW_BUILD_FRONTEND: build + CIBW_TEST_COMMAND: "python TESTS/unitTests.py" + + - name: "Upload wheel as artifact" + uses: actions/upload-artifact@v3 + with: + name: artifact-${{ matrix.os }}-wheel + path: "./**/*.whl" + + build-sdist: + name: Make source distribution + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - run: pipx run build --sdist + + - uses: actions/upload-artifact@v3 + with: + name: artifact-source-dist + path: "./**/dist/*.tar.gz" + deploy: + needs: [build_wheels, build-sdist] runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: '3.x' - - name: Install dependencies + - name: Download all artifacts + uses: actions/download-artifact@v3 + - name: Copy artifacts to dist/ folder run: | - python -m pip install --upgrade pip - pip install build - - name: Build package - run: python -m build --outdir dist/ --sdist + find . -name 'artifact-*' -exec unzip '{}' \; + mkdir -p dist/ + find . -name '*.tar.gz' -exec mv '{}' dist/ \; + find . -name '*.whl' -exec mv '{}' dist/ \; - name: Publish package to test pypi uses: pypa/gh-action-pypi-publish@release/v1 with: diff --git a/setup.py b/setup.py index 5381061..48783ce 100644 --- a/setup.py +++ b/setup.py @@ -1,5 +1,6 @@ #! /usr/bin/env python +from wheel.bdist_wheel import bdist_wheel from setuptools import setup, Extension import importlib import os @@ -11,6 +12,17 @@ pyrtools_version_spec.loader.exec_module(pyrtools_version_module) VERSION = pyrtools_version_module.version +# Adapted from the cibuildwheel example https://github.com/joerick/python-ctypes-package-sample +# it marks the wheel as not specific to the Python API version. +class WheelABINone(bdist_wheel): + def finalize_options(self): + bdist_wheel.finalize_options(self) + self.root_is_pure = False + + def get_tag(self): + _, _, plat = bdist_wheel.get_tag(self) + return "py3", "none", plat + setup( name='pyrtools', @@ -40,5 +52,6 @@ 'pyrtools/pyramids/c/convolve.h', 'pyrtools/pyramids/c/internal_pointOp.h'], extra_compile_args=['-fPIC', '-shared'])], + cmdclass={"bdist_wheel": WheelABINone}, tests='TESTS', ) From 12fadba0dab8ca34aa77699ec2ce17ccaecc6c45 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Fri, 15 Dec 2023 15:25:13 -0500 Subject: [PATCH 4/8] Update doc mentions of Windows support --- README.md | 10 +--------- docs/installation.rst | 7 +++---- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index f0ad43d..20ae4dc 100644 --- a/README.md +++ b/README.md @@ -41,15 +41,7 @@ repo](https://github.com/LabForComputationalVision/pyrtools) # Installation -It's recommended you install from pip: `pip install pyrtools`. The pip -install has been tested on Linux and on OSX. Windows is NOT supported -because of issues with the C compiler (`gcc` isn't necessarily -installed); if you have experience with C compilation on Windows, -please open a pull request. It's possible that the way to fix this is -to use Cython, ensuring that Cython is installed before attempting to -run the pip command, and then adding: `from Cython.Build import -cythonize` and wrapping the `ext_modules` in the `setup` call with -`cythonize`, but I'm not sure. +It's recommended you install from pip: `pip install pyrtools`. If you wish to install from the main branch, it's still recommended to use pip, just run `pip install .` (or `pip install -e .` if you diff --git a/docs/installation.rst b/docs/installation.rst index 2e0037f..60495c7 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -4,9 +4,8 @@ Installation ************ There are two ways to install pyrtools: via the `pip` package -management system, or directly from source. Note that we only support -Linux and macOS; we've had issues compiling the C code on Windows. If -you know how to fix this, please open up a Pull Request +management system, or directly from source. + (:ref:`dev-guide`). Note that we require `gcc` version >= 6 in order for the C code to compile, because of `this issue`_ @@ -28,7 +27,7 @@ Obtain the latest version of pyrtools:: Finally, the package is installed by running:: cd pyrtools - python setup.py -e . + pip install -e . This will install an editable version of the package, so changes made to the files within the pyrtools directory will be reflected in the From 9870978836de1d2e49c9f3b5b478c24115866ef7 Mon Sep 17 00:00:00 2001 From: "William F. Broderick" Date: Fri, 15 Dec 2023 17:10:29 -0500 Subject: [PATCH 5/8] some updates to install docs --- docs/installation.rst | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index 60495c7..a16ba16 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -3,12 +3,8 @@ Installation ************ -There are two ways to install pyrtools: via the `pip` package -management system, or directly from source. - -(:ref:`dev-guide`). Note that we require `gcc` version >= 6 in order -for the C code to compile, because of `this -issue`_ +There are two ways to install pyrtools: via the ``pip`` package management +system, or directly from source. Recommended =========== @@ -24,6 +20,8 @@ Obtain the latest version of pyrtools:: git clone https://github.com/LabForComputationalVision/pyrtools +(If you have already cloned the repo, you can update it with ``git pull``.) + Finally, the package is installed by running:: cd pyrtools @@ -32,3 +30,7 @@ Finally, the package is installed by running:: This will install an editable version of the package, so changes made to the files within the pyrtools directory will be reflected in the version of pyrtools you use. + +When installing from source on Linux or Mac, we require ``gcc`` version >= 6 in +order for the C code to compile, because of `this issue +`_ From 6f904ff8505ce2100436ca5e9f563a95de841298 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Mon, 18 Dec 2023 10:41:42 -0500 Subject: [PATCH 6/8] Allow manual building of wheels --- .github/workflows/deploy.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 8fa19f4..c9db857 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -2,6 +2,7 @@ name: deploy on: release: types: [published] + workflow_dispatch: {} jobs: build-wheels: @@ -26,7 +27,7 @@ jobs: CIBW_TEST_COMMAND: "python TESTS/unitTests.py" - name: "Upload wheel as artifact" - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: artifact-${{ matrix.os }}-wheel path: "./**/*.whl" @@ -39,7 +40,7 @@ jobs: - run: pipx run build --sdist - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: artifact-source-dist path: "./**/dist/*.tar.gz" @@ -47,10 +48,11 @@ jobs: deploy: needs: [build_wheels, build-sdist] runs-on: ubuntu-latest + if: github.event_name == 'release' && github.event.action == 'published' steps: - uses: actions/checkout@v3 - name: Download all artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 - name: Copy artifacts to dist/ folder run: | find . -name 'artifact-*' -exec unzip '{}' \; From 725537528a4125cbc594506476031b564cc32f06 Mon Sep 17 00:00:00 2001 From: "William F. Broderick" Date: Mon, 18 Dec 2023 11:39:10 -0500 Subject: [PATCH 7/8] adds Brian to CITATION.cff --- CITATION.cff | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CITATION.cff b/CITATION.cff index 97ddc0a..8a6b834 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -15,6 +15,8 @@ authors: given-names: Zahra - family-names: Parthasarathy given-names: Nikhil +- family-names: Ward + given-names: Brian orcid: https://orcid.org/0000-0000-0000-0000 title: "Pyrtools: tools for multi-scale image processing" version: v1.0.2 From 4b96db656e62cda90cf94135b6eb4e5dc2a10235 Mon Sep 17 00:00:00 2001 From: "William F. Broderick" Date: Mon, 18 Dec 2023 12:17:06 -0500 Subject: [PATCH 8/8] adds note about Windows C++ requirement. --- docs/installation.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/installation.rst b/docs/installation.rst index a16ba16..4e15bc1 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -34,3 +34,5 @@ version of pyrtools you use. When installing from source on Linux or Mac, we require ``gcc`` version >= 6 in order for the C code to compile, because of `this issue `_ + +When installing from source on Windows, Microsoft Visual C++ 14.0 or greater is required, which can be obtained with `Microsoft C++ Build Tools