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

GH-43519: [Python] Set up wheel building for Python 3.13 #43539

Merged
merged 35 commits into from
Aug 22, 2024

Conversation

lysnikolaou
Copy link
Contributor

@lysnikolaou lysnikolaou commented Aug 2, 2024

Rationale for this change

Like #43519 mentionies, now that the first rc is out, it's probably time to add CI coverage for Python 3.13 (and also start building wheels).

What changes are included in this PR?

I'm fairly new to the build/CI processes of the project, but I tried to follow the same template as #37901. I'll follow up afterwards with adding CI coverage for the free-threaded build as well.

Copy link

github-actions bot commented Aug 2, 2024

⚠️ GitHub issue #43519 has been automatically assigned in GitHub to PR creator.

@kou
Copy link
Member

kou commented Aug 2, 2024

@github-actions crossbow submit -g wheel

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Aug 2, 2024

This comment was marked as outdated.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Aug 5, 2024

@lysnikolaou thanks for the PR!

Looking at the manylinux wheel builds, that is failing for Python 3.13. While it is failing in the test step (about a docker image for 3.13 no being found, haven't yet looked in detail there), it's actually already failing to correctly build a py 3.13 wheel in the build step, because in practice it is building for Python 3.8 (-- Found Python3: /opt/python/cp38-cp38/bin/python (found version "3.8.18") found components: Interpreter Development.Module NumPy), I assume this is the default python in the manylinux docker image.

The docker image is based on the pypa images, but that is currently pinned to a version of earlier this year:

base: quay.io/pypa/manylinux2014_${ARCH_ALIAS}:2024-02-04-ea37246

That might need to be updated to a recent date (the latest builds in the pypa repo should include Python 3.13 I think). The last time this was updated: #39944

@jorisvandenbossche
Copy link
Member

@github-actions crossbow submit test-conda-python-3.13

Copy link

github-actions bot commented Aug 5, 2024

Revision: ed4e9d1

Submitted crossbow builds: ursacomputing/crossbow @ actions-404143b223

Task Status
test-conda-python-3.13 GitHub Actions

Comment on lines 1195 to 1196
{% for python_version in ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] %}
test-conda-python-{{ python_version }}:
Copy link
Member

Choose a reason for hiding this comment

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

I assume a conda build testing 3.13 will not yet work since conda-forge does not yet have Python 3.13. So maybe this change can be left out for now.

Also for a free-threaded variant, setting this up would not just work like this because then we need to install python from a custom channel? (https://py-free-threading.github.io/installing_cpython/#conda) But even then I am not sure if all other dependencies would then already have packages compatible with that python version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true. I removed 3.13 for now. Maybe it can be added later, when conda-forge does have a 3.13 version.

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 5, 2024
@lysnikolaou
Copy link
Contributor Author

Thanks for the review @jorisvandenbossche!

That might need to be updated to a recent date (the latest builds in the pypa repo should include Python 3.13 I think).

Yes, I missed those changes in the 3.12 PR. Just pushed a fix for that.

@jorisvandenbossche
Copy link
Member

@github-actions crossbow submit wheel-manylinux-*

This comment was marked as outdated.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Aug 6, 2024

The Mac failures are because of another issue (the py3.13 builds are failing with the same error as the others) -> #43416.

The windows failures seems related though ("Unable to resolve dependency 'python3 (= 3.13.0-rc1)'.")

@lysnikolaou
Copy link
Contributor Author

I missed the comment about update PYTHON_WHEEL_WINDOWS_IMAGE_REVISION in the env file. Maybe that's it?

@jorisvandenbossche
Copy link
Member

@github-actions crossbow submit wheel-windows-*

This comment was marked as outdated.

@jorisvandenbossche
Copy link
Member

The manylinux one is still failing with /opt/python/cp313-cp313t/bin:/opt/python/cp313-cp313: No such file or directory ...

Maybe the RUN PYTHON_ROOT=$(find /opt/python -name cp313-*) line in the dockerfile cannot handle that there are two python 3.13 directories?

@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

It's a bit strange that we see conda-based builds fail here with:

I see those Azurite-related errors from time to time, they seem entirely random to me :-/

@lysnikolaou
Copy link
Contributor Author

@lysnikolaou can you merge the latest main (or rebase on) once more?

Done! Is this ready to be merged?

@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

I'll file a separate PR for the "Dev / Lint" issues.

Edit: done in #43710

@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

For the record, the Azurite-related CI failures should be unrelated: #43702

Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

@github-actions crossbow submit cp313

Copy link

Revision: e12e2f5

Submitted crossbow builds: ursacomputing/crossbow @ actions-31c3bf8997

Task Status
wheel-macos-big-sur-cp313-arm64 GitHub Actions
wheel-macos-catalina-cp313-amd64 GitHub Actions
wheel-manylinux-2-28-cp313-amd64 GitHub Actions
wheel-manylinux-2-28-cp313-arm64 GitHub Actions
wheel-manylinux-2014-cp313-amd64 GitHub Actions
wheel-manylinux-2014-cp313-arm64 GitHub Actions
wheel-windows-cp313-amd64 GitHub Actions

@lysnikolaou
Copy link
Contributor Author

Not sure why wheel-manylinux-2014-cp313-arm64 failed, but the failure seems unrelated and it succeeded in the previous run.

@jorisvandenbossche
Copy link
Member

Not sure why wheel-manylinux-2014-cp313-arm64 failed, but the failure seems unrelated and it succeeded in the previous run.

And now it is also green after restarting it.

@jorisvandenbossche
Copy link
Member

NumPy 2.1.0 is released, so will update the requirements to use that version specifically.

@apache apache deleted a comment from github-actions bot Aug 22, 2024
@jorisvandenbossche
Copy link
Member

@github-actions crossbow submit cp313

@apache apache deleted a comment from github-actions bot Aug 22, 2024
Copy link

Revision: 747a630

Submitted crossbow builds: ursacomputing/crossbow @ actions-b98b471e60

Task Status
wheel-macos-big-sur-cp313-arm64 GitHub Actions
wheel-macos-catalina-cp313-amd64 GitHub Actions
wheel-manylinux-2-28-cp313-amd64 GitHub Actions
wheel-manylinux-2-28-cp313-arm64 GitHub Actions
wheel-manylinux-2014-cp313-amd64 GitHub Actions
wheel-manylinux-2014-cp313-arm64 GitHub Actions
wheel-windows-cp313-amd64 GitHub Actions

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM, let's hope wheel building succeeds

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Aug 22, 2024
@pitrou
Copy link
Member

pitrou commented Aug 22, 2024

Feel free to merge @jorisvandenbossche !

@jorisvandenbossche jorisvandenbossche merged commit 3e9384b into apache:main Aug 22, 2024
62 of 64 checks passed
@jorisvandenbossche jorisvandenbossche removed the awaiting changes Awaiting changes label Aug 22, 2024
@jorisvandenbossche
Copy link
Member

Thanks @lysnikolaou!

Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 3e9384b.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 13 possible false positives for unstable benchmarks that are known to sometimes produce them.

mapleFU pushed a commit to mapleFU/arrow that referenced this pull request Sep 3, 2024
…e#43539)

### Rationale for this change

Like apache#43519 mentionies, now that the first `rc` is out, it's probably time to add CI coverage for Python 3.13 (and also start building wheels).

### What changes are included in this PR?

I'm fairly new to the build/CI processes of the project, but I tried to follow the same template as apache#37901. I'll follow up afterwards with adding CI coverage for the free-threaded build as well.
* GitHub Issue: apache#43519

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants