add cibuildwheel GHA #1283

ocefpaf · 2023-10-16T17:28:36Z

This is just a proof of concept to see if we can build wheels here. We probably want to build out own hdf5 and netcdf-c b/c the ones from CentOS are quite old probably the source of the failures we are seeing in the tests here. See https://github.com/MacPython/netcdf4-python-wheels/blob/master/config.sh#L13-L26 for the versions used in the muiltbuild.

xref.: #1204

ocefpaf · 2023-10-16T17:39:12Z

Ubuntu wheels are using netcdf 4.3.3.1 and hdf5 1.8.12 is failing with:

 netCDF4 v1.6.5.
  .......E..........F.........s........................s...........................................
  ======================================================================
  ERROR: test_get (tst_fancyslicing.VariablesTestCase)
  testing 'fancy indexing'
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/project/test/tst_fancyslicing.py", line 147, in test_get
      data = v[ibx,iby,ibz]
    File "src/netCDF4/_netCDF4.pyx", line 4948, in netCDF4._netCDF4.Variable.__getitem__
    File "/tmp/tmp.QW23YtRUJ7/venv/lib/python3.8/site-packages/netCDF4/utils.py", line 429, in _StartCountStride
      start[...,i] = np.apply_along_axis(lambda x: e*x, i, np.ones(sdim[:-1]))
    File "<__array_function__ internals>", line 200, in apply_along_axis
    File "/tmp/tmp.QW23YtRUJ7/venv/lib/python3.8/site-packages/numpy/lib/shape_base.py", line 376, in apply_along_axis
      raise ValueError(
  ValueError: Cannot apply_along_axis when any iteration dimensions are 0
  
  ======================================================================
  FAIL: runTest (tst_masked.PrimitiveTypesTestCase)
  testing auto-conversion of masked arrays and packed integers
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/project/test/tst_masked.py", line 164, in runTest
      assert var2[:].mask.any() == False
  AssertionError

macOS is passing with the libs hdf5 1.14.2 and netcdf 4.9.2_1

richli · 2023-10-16T23:52:53Z

~~One option, I'm not sure if it's the best:~~

What about setting the cibuildwheel image to use manylinux_2_28? manylinux2014 is based on CentOS 7, which has less than a year left before its end-of-life (2024-06-30). manylinux_2_28 is based on Alma Linux 8, which uses netCDF 4.7.0.

But, in the existing wheel generation process (https://github.com/MacPython/netcdf4-python-wheels), netCDF and its dependencies are compiled from source, so currently netCDF 4.9.2 is being bundled for the wheels on PyPI.

~~So maybe a better option is to follow suit and build the dependencies from source in here as well instead of using the ancient versions in the EPEL 7 repos.~~

EDIT: I didn't read in the original description that you already suggested building the dependencies from source

ZedThree · 2023-10-19T12:22:14Z

I've done something similar for my nc-complex package: CI workflow, cibuildwheel config in the pyproject.toml, example workflow run

I used manylinux_2_28 in order to get the more recent HDF5, and then only had to build netcdf-C from source.

Note that I use my fork of netcdf-C because I'm waiting for PR #2762 to fix finding HDF5 correctly from CMake. Once that's in, using the lastest main of netcdf-C would be good because it will have compatibility with h5netcdf.

setup.py

ocefpaf · 2023-10-20T14:47:29Z

I'll try to focus on building hdf5 and netcdf-c over the next week There are a few options I want to try:

Use a more modern manylinux image and try the system libraries again. Advantage: easy. Disadvantage: little to no control over the hdf5 and netcdf-c;
Build using conda-forge packages for hdf5 and netcdf-c. This GitHub gave me this idea. Advantage: looks easier than the alternative 3 but harder than 1, uses community packages* that are curated by many maintainers who are also active here. Disadvantage: not 100% imediate control over the c-libs b/c someone needs to send a PR to conda-forge first;
Build both hdf5 and netcdf-c from source here. Advantage: 100% control over the builds. Disadvantage: hard to implement and maintain (IMO).

As a conda-forge founder, core member, and maintainer of many geo/met/ocean packages, I'm heavily biased towards 2. What do you think @ZedThree, @jswhit, and @dopplershift?

* we already do something similar for the Windows packages and I want to move the current Windows builds to this, regardless of the Linux and macOS choice, to keep things in a single repository.

ZedThree · 2023-10-20T15:15:48Z

I suggest using manylinux_2_28 with the system HDF5 (which is 1.10) and netcdf-C from source (because the system version is only 4.7.0).

I would definitely try avoiding building HDF5 from scratch if possible, as you might find you end up building a lot more of the stack!

ocefpaf · 2023-10-20T15:56:48Z

I suggest using manylinux_2_28 with the system HDF5 (which is 1.10) and netcdf-C from source (because the system version is only 4.7.0).

Looks like their hdf5 has libaec, that is a good start. I recall some issues with openssl in the past and that required folks to compiling their own. I'm not sure if that is still true. Let's call this suggestion would be "easy" to implement b/c I can copy your setup but would give 100% control only over the netcdf-c library. Somewhere between options 1 and 3.

jswhit · 2023-10-20T15:59:03Z

What about the approach of using conda built libs? (ocefpaf/netcdf4-win-wheels#6)

If you want to enable all the features of hdf5/netcdf-c it requires a lot of dependencies to be built - don't think we want to do all that here.

ocefpaf · 2023-10-20T16:02:02Z

What about the approach of using conda built libs? (ocefpaf/netcdf4-win-wheels#6)

If you want to enable all the features of hdf5/netcdf-c it requires a lot of dependencies to be built - don't think we want to do all that here.

Option 2 is a "modern" version of that where the main advantage is that it uses cibuildwheel+conda-forge clibs, and keep things in this repo, no need for third party ones. I'll try that and see if it works.

ocefpaf · 2023-10-20T23:38:17Z

.github/workflows/cibuildwheel.yml

+          CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: "delvewheel repair -w {dest_dir} {wheel}"
+          CIBW_TEST_COMMAND: >
+            python -c "import netCDF4; print(f'netCDF4 v{netCDF4.__version__}')"
+            && xcopy {project}\\test . /E/H


The {project} directory is on Drive D:, not C: and for some odd reason the PATH variables change if you switch drives on Windows. It is easier just to copy them to C:.

ocefpaf · 2023-10-20T23:39:11Z

.github/workflows/cibuildwheel.yml

+          CIBW_TEST_COMMAND: >
+            python -c "import netCDF4; print(f'netCDF4 v{netCDF4.__version__}')"
+            && xcopy {project}\\test . /E/H
+            && python -m pip install --upgrade numpy cython packaging


We can probably drop numpy, not sure why it wasn't installed with the wheel. Cython and packaging are test dependencies and we can figure out a more elegant way to handle this later.

Move them to pyproject.toml?

[project.optional-dependencies] tests = [ "Cython", "packaging", ]

ocefpaf · 2023-10-20T23:56:02Z

Windows wheels using conda package is working 🎉

I'll try the same for Linux and macOS next week. Time to clock out a bit.

ocefpaf · 2023-10-23T14:10:58Z

@jswhit everything is passing but the devil is in the details as usual. I'd love for more eyes here. We could also try to consolidate some of the many environment variables and options, netcdf-c version, etc but I prefer to leave that for another PR.

.ci/build_deps.sh

ocefpaf · 2023-10-23T17:45:36Z

.github/workflows/cibuildwheel.yml

@@ -70,7 +70,6 @@ jobs:
          && sh .ci/build_deps.sh
        CIBW_BEFORE_BUILD_MACOS: brew install hdf5 netcdf
        CIBW_TEST_SKIP: "*_arm64"
-        CIBW_TEST_REQUIRES: pytest cython


Looks like cibuildwheel still needs this regardless of it being declared in the pyporoject.toml. The levels of abstraction and isolation when building wheels is quite confusing. I guess we need to to a pip [test] install here. It has been to long that I used pip and I forget.

jswhit · 2023-10-23T23:47:24Z

This looks great! Thank you @ocefpaf and @ZedThree. I'd like to get this merged soon so we can get a 1.6.5 release out with 3.12 wheels. The old MacPython wheel building repo is broken, and it looks like the multibuild package it relies on is no longer actively maintained.

matthew-brett · 2023-10-24T07:25:07Z

The old MacPython wheel building repo is broken, and it looks like the multibuild package it relies on is no longer actively maintained.

Just to say - for the record - we (the multibuild maintainers) think the build is broken because your multibuild is not up to date (see MacPython/netcdf4-python-wheels#23) - and we are maintaining multibuild - but no complaints from us for switching away.

ocefpaf · 2024-06-10T19:55:18Z

.github/workflows/cibuildwheel.yml

+            CIBW_ENVIRONMENT: MACOSX_DEPLOYMENT_TARGET=14.0
+          - os: macos-13
+            arch: x86_64
+            CIBW_ENVIRONMENT: MACOSX_DEPLOYMENT_TARGET=13.0


These are a bit high, most folks set a lower bar with MACOSX_DEPLOYMENT_TARGET=10.9. If we aim to set 10.9 that we would need to build our own HDF5. I'm not sure how many people we are leaving behind by setting a higher deployment target, probably the same amount as brew. With that said, I'd like to try the HDF5 in another PR. This one is getting unwieldy already.

Looks like 13.0 was released in Oct 24, 2022 - this seems problematic to me. I think we would be leaving at least half our users behind with this restriction.

I have some guidelines on adding hdf5 from another project. I'll try that but I don't know if I can make it before the next release. Indeed, brew is too new.

OK I guess we could release 1.7.0 with the 13.0 restriction, and add new wheels to the pypi release later one the hdf5 build is working?

just a thought - macports supports older OSes than brew. Have no idea how hard that would be to get working.

Let's keep that as a plan B then. I'll send if I can get a better alternative working first. Pytable's infrastructure seems pretty close to what we need here.

Would MACOSX_DEPLOYMENT_TARGET 11 work for you? That is the oldest image available and brew is working fine to install both hdf5 and netcdf on it compiling from source. I still want to pursue the approach used in pytables at a later time. just want to be sure I'll have all the proper dependencies used in the current multibuild.

a target of macos 11 would be fine, yes.

a target of macos 11 would be fine, yes.

Done!

agree that 24M is pretty bloated, but I'd rather have that than restrict to macos 13.0

BTW, the main reason for that size was the inclusion of icu. We are discussing an alternative in conda-forge to remove that so we can make smaller wheels.

thanks a million @ocefpaf! Merging now...

test/run_all.py

ocefpaf · 2024-06-11T07:43:07Z

@ocefpaf I'm fine with the current approach for hdf5. Hope you enjoyed the long weekend!

@jswhit I'm done here! I guess that, the only future change I'd make is to build everything, hdf5, curl, openssl, etc from ourselves. But I'll leave that for the future when we have more time.

ocefpaf force-pushed the try_cibuildwheel branch 3 times, most recently from b228041 to 49a7d4e Compare October 16, 2023 17:31

ocefpaf force-pushed the try_cibuildwheel branch from be9c828 to 214cbb6 Compare October 20, 2023 13:59

ZedThree reviewed Oct 20, 2023

View reviewed changes

setup.py Outdated Show resolved Hide resolved

ocefpaf force-pushed the try_cibuildwheel branch 2 times, most recently from ac1a5ab to 7579696 Compare October 20, 2023 23:36

ocefpaf commented Oct 20, 2023

View reviewed changes

ocefpaf force-pushed the try_cibuildwheel branch 2 times, most recently from 5891d1b to 0b5a475 Compare October 20, 2023 23:43

ocefpaf mentioned this pull request Oct 20, 2023

Testing Windows wheels for Python 3.12 ocefpaf/netcdf4-win-wheels#10

Merged

ocefpaf force-pushed the try_cibuildwheel branch from 203bc1d to 371dc0f Compare October 22, 2023 22:36

ocefpaf marked this pull request as ready for review October 23, 2023 14:07

ocefpaf commented Oct 23, 2023

View reviewed changes

.ci/build_deps.sh Show resolved Hide resolved

ocefpaf force-pushed the try_cibuildwheel branch from f5bddfe to 63eae57 Compare October 23, 2023 14:14

ocefpaf commented Oct 23, 2023

View reviewed changes

ocefpaf force-pushed the try_cibuildwheel branch from 93eb57b to 6305503 Compare October 23, 2023 17:47

ocefpaf added 3 commits June 10, 2024 17:05

declare test deps

83dbdc7

Condense a bit

49ddb96

human readable sizes

ab4381f

ocefpaf force-pushed the try_cibuildwheel branch from 9014863 to ab4381f Compare June 10, 2024 15:05

pin ubuntu

dcce9e0

ocefpaf force-pushed the try_cibuildwheel branch from a245a9e to dcce9e0 Compare June 10, 2024 15:38

ocefpaf added 4 commits June 10, 2024 17:54

submodules

310fcf1

don't build aarch64 for now

3dbf4b1

increase verbosity

11b470f

increase verbosity, again

6f88d6a

ocefpaf force-pushed the try_cibuildwheel branch 2 times, most recently from d115346 to 57b50b6 Compare June 10, 2024 18:17

try to call pytest on Windows instead

2cb4c25

ocefpaf force-pushed the try_cibuildwheel branch 3 times, most recently from 03a9078 to 9adbfa4 Compare June 10, 2024 19:13

set MACOSX_DEPLOYMENT_TARGET

33eebb9

ocefpaf force-pushed the try_cibuildwheel branch from 9adbfa4 to 33eebb9 Compare June 10, 2024 19:20

make sure all artifacts are available for upload

db5b321

ocefpaf force-pushed the try_cibuildwheel branch from 6ebf4e6 to db5b321 Compare June 10, 2024 19:48

ocefpaf commented Jun 10, 2024

View reviewed changes

test/run_all.py Show resolved Hide resolved

ocefpaf added 3 commits June 10, 2024 22:04

use pytest instead of run_all for nix

d5521bf

fix list artifacts

78d1c4f

reduce matrix load on CIs

1452d90

ocefpaf force-pushed the try_cibuildwheel branch from 5ab5403 to 1452d90 Compare June 11, 2024 07:33

use oldest image possible for intel macs

73d1890

jswhit merged commit 7755fd1 into Unidata:master Jun 13, 2024
33 checks passed

ocefpaf deleted the try_cibuildwheel branch June 13, 2024 07:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cibuildwheel GHA #1283

add cibuildwheel GHA #1283

ocefpaf commented Oct 16, 2023 •

edited

Loading

ocefpaf commented Oct 16, 2023 •

edited

Loading

richli commented Oct 16, 2023 •

edited

Loading

ZedThree commented Oct 19, 2023

ocefpaf commented Oct 20, 2023 •

edited

Loading

ZedThree commented Oct 20, 2023

ocefpaf commented Oct 20, 2023

jswhit commented Oct 20, 2023

ocefpaf commented Oct 20, 2023

ocefpaf Oct 20, 2023

ocefpaf Oct 20, 2023

ZedThree Oct 23, 2023

ocefpaf commented Oct 20, 2023 •

edited

Loading

ocefpaf commented Oct 23, 2023

ocefpaf Oct 23, 2023 •

edited

Loading

jswhit commented Oct 23, 2023

matthew-brett commented Oct 24, 2023 •

edited

Loading

ocefpaf Jun 10, 2024

jswhit2 Jun 11, 2024

ocefpaf Jun 11, 2024

jswhit2 Jun 11, 2024

jswhit2 Jun 11, 2024

ocefpaf Jun 11, 2024

ocefpaf Jun 12, 2024

jswhit2 Jun 12, 2024

ocefpaf Jun 12, 2024

jswhit2 Jun 13, 2024

ocefpaf commented Jun 11, 2024

add cibuildwheel GHA #1283

add cibuildwheel GHA #1283

Conversation

ocefpaf commented Oct 16, 2023 • edited Loading

ocefpaf commented Oct 16, 2023 • edited Loading

richli commented Oct 16, 2023 • edited Loading

ZedThree commented Oct 19, 2023

ocefpaf commented Oct 20, 2023 • edited Loading

ZedThree commented Oct 20, 2023

ocefpaf commented Oct 20, 2023

jswhit commented Oct 20, 2023

ocefpaf commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocefpaf commented Oct 20, 2023 • edited Loading

ocefpaf commented Oct 23, 2023

ocefpaf Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

jswhit commented Oct 23, 2023

matthew-brett commented Oct 24, 2023 • edited Loading

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

ocefpaf commented Jun 11, 2024

ocefpaf commented Oct 16, 2023 •

edited

Loading

ocefpaf commented Oct 16, 2023 •

edited

Loading

richli commented Oct 16, 2023 •

edited

Loading

ocefpaf commented Oct 20, 2023 •

edited

Loading

ocefpaf commented Oct 20, 2023 •

edited

Loading

ocefpaf Oct 23, 2023 •

edited

Loading

matthew-brett commented Oct 24, 2023 •

edited

Loading