Add wheel building and deployment via GitHub Actions and cibuildwheel #601

grlee77 · 2021-10-06T06:15:03Z

This PR is adapted from scikit-image/scikit-image#5397.

Apparently we had added pyproject.toml here long ago and then removed it due to some issues with pip at that time. This PR reintroduces it so that the correct NumPy version will be used during these builds (it is based on SciPy's, but without the Pythran or Pybind11 requirements).

This also has automated deployment when pushing a version tag. If the automated deploy fails, we can still retrieve the wheels from the artifacts.

comment out the deployment steps for now while testing add missing dependency installs

comment out pip install of cython. should get picked up from pyproject.toml comment out CPython 3.10 build on Windows and MacOS

grlee77 · 2021-10-06T06:27:57Z

there are a lot of unrelated test failures related to coverage that I will have to take a look at.

The status of the wheel builds can be seen in my fork

grlee77 · 2021-10-06T06:31:41Z

Could use feedback on which manylinux we should be using (many cases are still manylinux1).

rgommers

Thanks @grlee77, this looks great! Happy to copy this approach and drop MacPython.

+1 for copying SciPy's pyproject.toml approach rather than using oldest-supported-numpy. It's slightly more work, but allows to use higher minimum numpy versions and thereby possible avoiding some C API/ABI issues.

Could use feedback on which manylinux we should be using (many cases are still manylinux1).

We can go manylinux2014-only soon, numpy has dropped the rest in its main branch already. But we'd rather not be the first ones to actually do this probably, so can be left to a separate PR after numpy et al. have done it.

rgommers · 2021-10-06T08:12:22Z

pyproject.toml

+    # we specify an unpinned NumPy which allows source distributions
+    # to be used and allows wheels to be used as soon as they
+    # become available.
+    "numpy; python_version>='3.10'",


Should we add 3.10 now, or leave that to a quick follow-up PR? We need 3.10 wheels, forgot about those till this PR.

Having a look at this now.

.github/workflows/wheel_tests_and_release.yml

rgommers · 2021-10-06T08:19:15Z

.github/workflows/wheel_tests_and_release.yml

+      - name: Set up QEMU
+        uses: docker/setup-qemu-action@v1
+        with:
+          platforms: arm64


This reminds me: we're going to need macOS arm64 too soon. Best to leave for a separate PR. Although would be good to check that it's possible with cibuildwheel or in the works (the multibuild support is available now).

It looks like macOS arm64 should be possible for Python 3.8, 3.9 and 3.10: https://github.com/pypa/cibuildwheel#what-does-it-do

I added universal2 and arm64 wheels on MacOS and it succeeded (see: https://github.com/grlee77/pywt/actions/runs/1334259199). Do you recommend providing the universal2 wheels in addition to the separate x86_64 and arm64 ones?

Yes, universal2 is useful I think, since it's the default for the Python.org installers and because it's the best wheel to grab if you don't know beforehand what you need.

rgommers · 2021-10-06T08:25:21Z

.github/workflows/wheel_tests_and_release.yml

+      - cibuildwheel
+    tags:
+      - 'v*'
+      - 'buildwheels*'


What is this for? There seems to be no guard for this kind of tag anywhere, so does it upload a release for them to PyPI without a normal version number?

Same question for the cibuildwheel branch name.

We can remove the cibuildwheel branch condition before merging. That was just to make it run everytime I push an update while testing.

Pushing a buildwheels* tag should not trigger deployment. I think the following conditions requires the tag start with v. I assume the repository_owner check is to prevent it running on forks.

deploy: name: Release needs: [build_linux_37_and_above_wheels, build_macos_wheels, build_windows_wheels] if: github.repository_owner == 'PyWavelets' && startsWith(github.ref, 'refs/tags/v') && always()

I think the needs section ensures that it only gets run if the wheel builds all succeeded.

this can be removed in a follow-up PR, it's harmless to have in here.

rgommers · 2021-10-06T08:32:04Z

These jobs are crazy slow:

Looks like they're all using QEMU instead of only the arm64 job - I hope that is not on purpose?

grlee77 · 2021-10-06T14:45:27Z

Looks like they're all using QEMU instead of only the arm64 job - I hope that is not on purpose?

It seems to be just running tests on aarch64 that is taking 80-90% of the time.

Example for x86_64

 Building cp39-manylinux_x86_64 wheel
CPython 3.9 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.11s
Building wheel...
                                                             ✓ 54.20s
Repairing wheel...
                                                              ✓ 1.49s
Testing wheel...
                                                             ✓ 96.92s

but for aarch64


Building cp39-manylinux_aarch64 wheel
CPython 3.9 manylinux aarch64

Setting up build environment...
                                                              ✓ 1.09s
Building wheel...
                                                            ✓ 644.47s
Repairing wheel...
                                                             ✓ 10.83s
Testing wheel...
                                                           ✓ 3253.64s

For some reason on Python 3.10 the aarch64 tests took even longer (2.5 hours!)

grlee77 · 2021-10-06T14:51:17Z

Those 4 linux jobs in the screenshot are divided among Python versions. But each version runs on three architextures (x86_64, i686 and aarch64). It is mainly the aarch64 one that is taking a long time.

Perhaps we can conditionally skip running the full test suite on aarch64 and just run a simple test script checking the package import and a few simple calls.

rgommers · 2021-10-06T15:49:53Z

each version runs on three architextures (x86_64, i686 and aarch64). It is mainly the aarch64 one that is taking a long time.

Ah that makes sense.

Perhaps we can conditionally skip running the full test suite on aarch64 and just run a simple test script checking the package import and a few simple calls.

If it's just once per release, then it's not a problem. But if it runs more often, I agree we should skip it by default.

grlee77 · 2021-10-06T18:01:29Z

If it's just once per release, then it's not a problem. But if it runs more often, I agree we should skip it by default.

Yeah, it is just for releases (or if we push a buildwheel tag to test things). Let's just leave it as-is. IIRC, the timeout is 5 or 6 hours, so we are still not too close to that.

to avoid warning about --include ignored because --source is set

rgommers · 2021-10-12T18:52:15Z

.github/workflows/tests.yml

@@ -20,7 +20,7 @@ jobs:
      # Ensure that a wheel builder finishes even if another fails
      fail-fast: false
      matrix:
-        python-version: [3.7, 3.8, 3.9]
+        python-version: [3.7, 3.8, 3.9, 3.10]


This should be "3.10" (with quotes). Now it causes:

Version 3.1 was not found in the local cache Error: Version 3.1 with arch x64 not found

Thanks, I fixed that, but then ran into a problem with pip trying to install Matplotlib on OS X with 3.10. I assume that will resolve itself once Matplotlib makes a release with python 3.10 wheels. I should perhaps remove the addition of Python 3.10 tests.yml from this PR and leave that to a follow-up.

Separating out the Python 3.10 support sounds good. We can leave Matplotlib out for 3.10 probably - we don't need it to make a release.

3.10 was interpreted as 3.1 and failed

will added it to both tests and wheel building in a separate PR

grlee77 · 2021-10-24T17:51:23Z

This is probably ready to go unless you want me to modify which wheels get built on macOS. I think some other projects are shipping only the universal2 wheels, so do we want to do the same?

I removed 3.10 from this PR and will and it in a follow-up.

rgommers · 2021-10-24T20:34:51Z

This is probably ready to go unless you want me to modify which wheels get built on macOS. I think some other projects are shipping only the universal2 wheels, so do we want to do the same?

I think thin wheels are good to have, they are smaller and therefore should be preferred over universal2 in the long run. The choice for universal2 in Python packaging was mostly out of convenience (less changes to make because the old universal support was already present), but from a technical perspective it doesn't make too much sense to almost double the size of wheels for some convenience.

rgommers

LGTM, let's give this a go. Thanks a lot @grlee77!

grlee77 added 4 commits October 6, 2021 02:07

add wheel_tests_and_release.yml (adapted from scikit-image)

fcf43e6

comment out the deployment steps for now while testing add missing dependency installs

add cpython 3.10 builds

7c906d1

add pyproject.toml to get numpy version

6ad4da6

need to use manylinux2014 for Python 3.10

0b71d75

comment out pip install of cython. should get picked up from pyproject.toml comment out CPython 3.10 build on Windows and MacOS

grlee77 added the build label Oct 6, 2021

rgommers reviewed Oct 6, 2021

View reviewed changes

rgommers added this to the v1.2 milestone Oct 6, 2021

grlee77 added 6 commits October 12, 2021 12:17

remove include entry from .coveragerc

4956ec2

to avoid warning about --include ignored because --source is set

try bumping to Python 3.9 in cibuildwheel workflow

8920a3e

I think Python 3.5 is now already skipped by default

d74ed02

add Python 3.7 entry to pyproject.toml

88b74a5

add Python 3.10 to GitHub Actions test cases

6d95a86

add CIBW_ARCHS_MACOS

824b4c8

rgommers reviewed Oct 12, 2021

View reviewed changes

grlee77 added 3 commits October 12, 2021 15:21

try specifying '3.10' instead of 3.10

2a670fc

3.10 was interpreted as 3.1 and failed

remove Python 3.10 for now

3eb4cba

will added it to both tests and wheel building in a separate PR

remove cibuildwheel branch condition [skip CI]

4005ec0

rgommers approved these changes Oct 24, 2021

View reviewed changes

rgommers merged commit 2c3910f into PyWavelets:master Oct 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wheel building and deployment via GitHub Actions and cibuildwheel #601

Add wheel building and deployment via GitHub Actions and cibuildwheel #601

grlee77 commented Oct 6, 2021

grlee77 commented Oct 6, 2021

grlee77 commented Oct 6, 2021

rgommers left a comment

rgommers Oct 6, 2021

rgommers Nov 6, 2021

rgommers Oct 6, 2021

grlee77 Oct 12, 2021

grlee77 Oct 12, 2021

rgommers Oct 24, 2021

rgommers Oct 6, 2021

grlee77 Oct 6, 2021

rgommers Oct 24, 2021

rgommers commented Oct 6, 2021

grlee77 commented Oct 6, 2021 •

edited

grlee77 commented Oct 6, 2021

rgommers commented Oct 6, 2021

grlee77 commented Oct 6, 2021

rgommers Oct 12, 2021

grlee77 Oct 12, 2021

rgommers Oct 13, 2021

grlee77 commented Oct 24, 2021

rgommers commented Oct 24, 2021

rgommers left a comment

Add wheel building and deployment via GitHub Actions and cibuildwheel #601

Add wheel building and deployment via GitHub Actions and cibuildwheel #601

Conversation

grlee77 commented Oct 6, 2021

grlee77 commented Oct 6, 2021

grlee77 commented Oct 6, 2021

rgommers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers commented Oct 6, 2021

grlee77 commented Oct 6, 2021 • edited

grlee77 commented Oct 6, 2021

rgommers commented Oct 6, 2021

grlee77 commented Oct 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grlee77 commented Oct 24, 2021

rgommers commented Oct 24, 2021

rgommers left a comment

Choose a reason for hiding this comment

grlee77 commented Oct 6, 2021 •

edited