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-45071: [Packaging][Docs] Fix NumPy v2 include directory for Emscripten, and update Pyodide-related documentation #45072

Merged
merged 5 commits into from
Dec 20, 2024

Conversation

agriyakhetarpal
Copy link
Contributor

@agriyakhetarpal agriyakhetarpal commented Dec 18, 2024

Rationale for this change

This change would allow building PyArrow correctly with NumPy 1.X and NumPy 2.X, since we are trying to do the latter for pyodide/pyodide#4925. This PR closes gh-45071.

What changes are included in this PR?

This PR

  • issues a correction for the NumPy header files when building under Emscripten
  • updates Pyodide-related build instructions

Are these changes tested?

Yes, working here: https://github.com/agriyakhetarpal/pyodide-numpy-2.0-rebuilds/actions/runs/12399351376/job/34619554658#step:8:4547 via agriyakhetarpal/pyodide@b651698 that applies a subset of the changes as a patch (the CI job is failing for unrelated reasons, please ignore).

Are there any user-facing changes?

Yes, users trying to build a WASM wheel via Pyodide are now requested to use newer Pyodide and Emscripten versions, and the latest stable version of pyodide-build available.

Copy link

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

@kou
Copy link
Member

kou commented Dec 19, 2024

@github-actions crossbow submit -g python

Copy link

Revision: bb6ff29

Submitted crossbow builds: ursacomputing/crossbow @ actions-a9ba1a0870

Task Status
example-python-minimal-build-fedora-conda GitHub Actions
example-python-minimal-build-ubuntu-venv GitHub Actions
test-conda-python-3.10 GitHub Actions
test-conda-python-3.10-cython2 GitHub Actions
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions
test-conda-python-3.10-pandas-latest-numpy-latest GitHub Actions
test-conda-python-3.10-substrait GitHub Actions
test-conda-python-3.11 GitHub Actions
test-conda-python-3.11-dask-latest GitHub Actions
test-conda-python-3.11-dask-upstream_devel GitHub Actions
test-conda-python-3.11-hypothesis GitHub Actions
test-conda-python-3.11-pandas-latest-numpy-1.26 GitHub Actions
test-conda-python-3.11-pandas-latest-numpy-latest GitHub Actions
test-conda-python-3.11-pandas-nightly-numpy-nightly GitHub Actions
test-conda-python-3.11-pandas-upstream_devel-numpy-nightly GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.12 GitHub Actions
test-conda-python-3.12-cpython-debug GitHub Actions
test-conda-python-3.13 GitHub Actions
test-conda-python-3.9 GitHub Actions
test-conda-python-3.9-pandas-1.1.3-numpy-1.19.5 GitHub Actions
test-conda-python-emscripten GitHub Actions
test-cuda-python-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-python-3-amd64 GitHub Actions
test-debian-12-python-3-i386 GitHub Actions
test-fedora-39-python-3 GitHub Actions
test-ubuntu-22.04-python-3 GitHub Actions
test-ubuntu-22.04-python-313-freethreading GitHub Actions
test-ubuntu-24.04-python-3 GitHub Actions

@agriyakhetarpal
Copy link
Contributor Author

Thanks for triggering CI! Looks like the failure is unrelated, it's just a timeout.

@raulcd
Copy link
Member

raulcd commented Dec 19, 2024

Thanks for triggering CI! Looks like the failure is unrelated, it's just a timeout.

Yes, it seems a timeout issue but it's pretty consistent, I just triggered it a 4th time, and it has never failed on our nightly CI. The other CI failures are unrelated.

edit: Well, it was successful on the 4th attempt :)

@agriyakhetarpal
Copy link
Contributor Author

Glad that it passed :D

I would recommend switching to using the Pyodide virtual environment pyodide venv for running the test suite, by the way – while python/scripts/run_emscripten_tests.py tests on Chrome and Firefox as well, the Node.js tests in it could be swapped out for it, and we found to be more reliable in projects such as SymPy.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

I did more tries and they are stable now. (Why...?)

Could you open new issues for these improvement ideas?

@kou kou merged commit 7bec4fa into apache:main Dec 20, 2024
34 checks passed
@kou kou removed the awaiting review Awaiting review label Dec 20, 2024
@agriyakhetarpal
Copy link
Contributor Author

I have no idea either, though FWIW, timeouts and flakes with selenium in our own test suite are pretty common.

I'll open a new issue about it, sure!

@agriyakhetarpal agriyakhetarpal deleted the fix/pyodide-builds branch December 20, 2024 08:58
Copy link

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

There were no benchmark performance regressions. 🎉

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants