Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Drop cibuildwheel matrix generation #2450

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 7, 2024

Rationale

We used to install cibuildwheel manually in order to generate the Python-version matrix. This means that dependabot does not notice or update it. This is normally fine, but when cibuildwheel adds new Python versions, they won't be in the generated matrix until the pin is updated.

Since our builds are relatively quick, drop the job matrix generation and just split by platform/arch.

Also, expand the jq arguments so it's a bit clearer what it does, and limit workflow permissions.

Implications

This should add Python 3.13 wheels to the matrix.

@CLAassistant
Copy link

CLAassistant commented Oct 7, 2024

CLA assistant check
All committers have signed the CLA.

@QuLogic QuLogic changed the title c: Fix pin for cibuildwheel when building matrix ci: Fix pin for cibuildwheel when building matrix Oct 7, 2024
@QuLogic QuLogic added the CI: build wheels Run the release/wheel building on PR label Oct 7, 2024
@QuLogic
Copy link
Member Author

QuLogic commented Oct 7, 2024

I see I haven't contributed in a long while considering I signed the CLA when it was v4.

@QuLogic QuLogic marked this pull request as draft October 7, 2024 23:18
@QuLogic QuLogic force-pushed the py313-wheels branch 2 times, most recently from af4d2c0 to 6f5d850 Compare October 8, 2024 01:42
Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:( Sorry, I could have sworn I checked 3.13 wheels earlier but I must have been looking at the test matrix instead. Thanks for the really quick investigation and fix here!

I wonder if this is more fancy than necessary and we should just do the serial builds on each OS like most other projects...

Looks like all tests and builds are green. Can you merge and make a new release when you're ready?

@QuLogic
Copy link
Member Author

QuLogic commented Oct 8, 2024

Yes, this does do a bit of a weird thing in that it assumes the location that cibuildwheel gets installed. Builds take about 0.5-2 minutes each (unlike, e.g., Matplotlib's 10-60 for all versions combined in its matrix), so maybe it's just easier to simplify to per-arch/platform?

We used to install cibuildwheel manually in order to generate the
Python-version matrix. This means that dependabot does not notice or
update it. This is normally fine, but when cibuildwheel adds new Python
versions, they won't be in the generated matrix until the pin is
updated.

Since our builds are relatively quick, drop the job matrix generation
and just split by platform/arch.

Also, expand the jq arguments so it's a bit clearer what it does, and
limit workflow permissions.
@QuLogic
Copy link
Member Author

QuLogic commented Oct 8, 2024

Do we want to enable free-threading wheels as well? Or possibly PyPy too?

@greglucas
Copy link
Contributor

Do we want to enable free-threading wheels as well? Or possibly PyPy too?

  • free-threading, I tried building locally and I don't think we can quite yet because Shapely and Pyproj both need to be built from source for it at the moment, so probably not worth doing until they have wheels we can build with (we use them within the Cython code, so need it for building our extension).
  • PyPy, I don't use personally, so if others would find it useful then sure 🤷 , but I would wait until we get a request for it before spending much time getting that to work.

All in all, I'd suggest just going with your minimal change here. I like the simplification of the build too, thanks for consolidating that!

@QuLogic QuLogic changed the title ci: Fix pin for cibuildwheel when building matrix ci: Drop cibuildwheel matrix generation Oct 8, 2024
@QuLogic QuLogic marked this pull request as ready for review October 8, 2024 20:45
@QuLogic QuLogic added this to the Next Release milestone Oct 8, 2024
@QuLogic
Copy link
Member Author

QuLogic commented Oct 8, 2024

Looks like all tests and builds are green. Can you merge and make a new release when you're ready?

I'm just running tests on Fedora now, so won't release yet, just in case there's some followup there.

@QuLogic QuLogic merged commit 1c704d1 into SciTools:main Oct 8, 2024
26 checks passed
@QuLogic QuLogic deleted the py313-wheels branch October 8, 2024 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: build wheels Run the release/wheel building on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants