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

BLD, CI: Use cibuildwheel to build Emscripten/Pyodide wheels, push nightlies to Anaconda.org #58647

Merged
merged 20 commits into from
Jul 12, 2024

Conversation

agriyakhetarpal
Copy link
Contributor

@agriyakhetarpal agriyakhetarpal commented May 8, 2024

Description

This PR duplicates the Pyodide wheel builder job that was set up in #57896 into wheels.yml, except that in this case the wheel is not tested and the wheel builder uses cibuildwheel enabled with Pyodide to build the wheels. Subsequently, the upload_wheels.sh script will be used with the appropriate repository secret to upload the Pyodide wheel to https://anaconda.org/scientific-python-nightly-wheels/pandas.

Additional context

  1. This is how this was done for NumPy: CI, BLD: Push NumPy's Emscripten/Pyodide wheels nightly to Anaconda.org PyPI index numpy/numpy#26134
  2. At a later time, these wheels will be used to configure interactive documentation via JupyterLite through jupyterlite-sphinx that acts upon doctest-based examples. The blocker in that regard is setting up a method to pre-install these wheels to provide them to the jupyterlite-pyodide-kernel and make them available at run time, and progress is being made in this area.

Adding an xref for tracking purposes, the action from main will be used until a stable release of cibuildwheel is available that can build Pyodide wheels: pypa/cibuildwheel#1456

.github/workflows/wheels.yml Outdated Show resolved Hide resolved
.github/workflows/wheels.yml Outdated Show resolved Hide resolved
.github/workflows/wheels.yml Outdated Show resolved Hide resolved
@agriyakhetarpal agriyakhetarpal changed the title BLD: BLD, CI: Upload Emscripten/Pyodide nightlies to Anaconda index (scientific-python-nightly-wheels) BLD, CI: Upload Emscripten/Pyodide nightlies to Anaconda index (scientific-python-nightly-wheels) May 8, 2024
@agriyakhetarpal agriyakhetarpal changed the title BLD, CI: Upload Emscripten/Pyodide nightlies to Anaconda index (scientific-python-nightly-wheels) BLD, CI: Upload Emscripten/Pyodide nightlies to Anaconda index (scientific-python-nightly-wheels) May 8, 2024
@lithomas1 lithomas1 added the Build Library building on various platforms label May 16, 2024
@lithomas1
Copy link
Member

Looks like cibuildwheel might be adding pyodide here
pypa/cibuildwheel#1456

Maybe we should wait for that PR?

@agriyakhetarpal
Copy link
Contributor Author

agriyakhetarpal commented May 16, 2024

Hi @lithomas1, I would advise holding off on that PR – it's stalled for many reasons, for example the fact that Pyodide does releases bi-annually with updated Python versions and Emscripten versions which will break the ABI – i.e., while the specification for breaking changes and ABI stability for the platform might exist, it has not been formalised through the means of a PEP yet; and because PyPI has not yet received support through a PEP for formalising the wasm32 tag through packaging and pip, which means that micropip remains the only choice for installing these wheels (which, then, does not come with resolvelib, AFAIK) and wheels need to be pushed to alternative indices – Anaconda.org, in this case. However, out-of-tree build support in the ecosystem is improving day by day; it could be a viable candidate sometime soon. In the meantime, this current method of building should be a reasonable alternative since it's tightly integrated into and similar to Pyodide's in-tree builds with the pyodide build CLI.

@agriyakhetarpal
Copy link
Contributor Author

I ended up asking around on that PR and it has been merged now after more than a year of effort; cibuildwheel hasn't had a release with it yet, but following these recent developments I have tried it for NumPy and it works nicely: numpy/numpy#26570 – let me replace the configuration here as well and rebase this branch.

I would still recommend using this existing configuration because it's much more stable and it will allow us to get these wheels uploaded to Anaconda.org much quicker, but in any case, that's not a high-priority pressing issue right now, so I would not mind if there is a delay with that and I understand that the review process can take time, running on its own accord.

@agriyakhetarpal agriyakhetarpal changed the title BLD, CI: Upload Emscripten/Pyodide nightlies to Anaconda index (scientific-python-nightly-wheels) BLD, CI: Use cibuildwheel to build Emscripten/Pyodide wheels, push nightlies to Anaconda.org May 30, 2024
@agriyakhetarpal
Copy link
Contributor Author

Both the wheel build and the tests pass 🎉: https://github.com/pandas-dev/pandas/actions/runs/9307939926/job/25620279093

I'll convert this to a draft for now – I will mark it as ready for review as soon as a stable cibuildwheel release hits.

@agriyakhetarpal
Copy link
Contributor Author

agriyakhetarpal commented Jun 11, 2024

Based on https://github.com/pandas-dev/pandas/actions/runs/9466987065/job/26080112499, I think there are some failing tests that are not running in the regular Emscripten job, and I am not sure why yet. I'll fix them as necessary in subsequent commits.

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review June 11, 2024 15:06
@agriyakhetarpal
Copy link
Contributor Author

Ready for review, @mroeschke and @lithomas1.

@WillAyd
Copy link
Member

WillAyd commented Jun 18, 2024

I am -1 on adding this to CI or claiming any official support; this just makes a more Arrow-centric future even more difficult

@agriyakhetarpal
Copy link
Contributor Author

@WillAyd, thanks for your comment. Could a reasonable alternative be to upload these nightly wheels silently without an announcement or a CHANGELOG entry, at least for now? It has been a while since the Pyodide job was added in #57896 and I figure that it has been running smoothly without any outstanding errors – I don't see anything new having turned up in issues or PRs with either "Emscripten" or "Pyodide" being referenced :)

Moreover, apache/arrow#37822 just got merged a few days ago, and though pyodide/pyodide#2933 isn't going to get into Pyodide 0.26.2, it will be going into Pyodide 0.27.X later in the year, marking official Arrow support in/for Pyodide. Though I understand and I am aware of the PDEP and the ramifications by now on bandwidth use and the drastic increase in download sizes, an Arrow-centric future might be reasonably possible and is definitely getting closer. I hope that you can reconsider your stance!

@WillAyd
Copy link
Member

WillAyd commented Jul 10, 2024

@agriyakhetarpal that's great news on the pyarrow support. With that being the case I have no concerns

@mroeschke mroeschke added the CI Continuous Integration label Jul 10, 2024
Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>
@lithomas1 lithomas1 self-requested a review July 10, 2024 17:10
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

LGTM (failures look unrelated) cc @lithomas1 if you want a look

@agriyakhetarpal
Copy link
Contributor Author

@lithomas1, I did what you requested and the workflow runs on my fork successfully: https://github.com/agriyakhetarpal/pandas/actions/runs/9905625217/job/27365583191. @mroeschke, this may slightly contradict your previous suggestion in #58647 (comment) about cibuildwheel taking care of future Python versions, because I have hardcoded the Python version (3.12) again here for the build identifier – however, cibuildwheel isn't going to bump the Pyodide version anytime soon, since there is considerable time with the semiannual release approach for Pyodide. Please feel free to tag me if anything breaks, because, as the one who has proposed this addition to this workflow, I intend to maintain it too and keep it updated. This is now ready for merging.

agriyakhetarpal and others added 2 commits July 12, 2024 23:23
Co-Authored-By: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Co-Authored-By: Thomas Li <47963215+lithomas1@users.noreply.github.com>
@mroeschke mroeschke added this to the 3.0 milestone Jul 12, 2024
@mroeschke mroeschke merged commit 2a9855b into pandas-dev:main Jul 12, 2024
79 of 82 checks passed
@mroeschke
Copy link
Member

Thanks @agriyakhetarpal

@agriyakhetarpal agriyakhetarpal deleted the upload-emscripten-wheels branch July 12, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants