-
Notifications
You must be signed in to change notification settings - Fork 98
Add out-of-tree Pyodide builds in CI for numcodecs
#529
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
base: main
Are you sure you want to change the base?
Add out-of-tree Pyodide builds in CI for numcodecs
#529
Conversation
This commit makes the following changes: 1. Formatting and whitespace changes 2. Removes outdated Python version references and adds references to the current supported version of Python for NumCodecs 3. Removes references to Travis and AppVeyor since GitHub Actions is being used in favour of them as a CI provider.
numcodecs
numcodecs
numcodecs
numcodecs
The workflow is currently going to fail because the file system access in Emscripten has a bug with saving @rgommers and I had fixed a similar error in PyWavelets/pywt#701 by converting all of the |
Thanks, @agriyakhetarpal. I've launched the workflows. |
Ah, I think the |
Never mind, looks like #528 is already on it – I can wait and rebase later after that gets merged. For the relevant jobs here, the Pyodide wheel build succeeds while the tests fail because of the reasons described above, and the rest of the test suite passes. |
Marking this PR as a draft till there is a fix for this on the Pyodide and Emscripten ends. I will put my focus on an accompanying CI job and associated fixes for the Zarr repository. |
It's been a while since this PR has been opened, could we get the ball rolling by temporarily skipping the previously failing test for now? I feel that |
We've cut a |
It looks like #569 is going to change a few things here – I have to note that Pyodide will need to set the provisioned I'm still happy to discuss with upstream maintainers if they're interested in such patches as I mentioned above in this PR, and I assume it helps both them and |
(Relaunched) |
Thanks, I fixed the drop in the coverage – it's just that Codecov doesn't know about this WASM environment setup, so it can't be aware that this code path is being tested elsewhere. |
Re-running jobs after:
|
I've addressed the merge conflicts introduced in the release notes file at the time of the 0.16 release. Thanks! I wanted to note that there is a recent PR at #737, which also concerns the |
Hi! I've resolved the new merge conflicts that had sprung up here. Also, I made sure that we're testing against the same Pyodide version as we're building the WASM wheels for. If you'd like me to, I can also try against the latest version of Pyodide to check for any regressions (which will use Emscripten 4.0.9 instead of 3.1.58) – cibuildwheel v3 soon will help doing this (i.e., testing against nightly or alpha versions of Pyodide), and a beta (3.0.0b1) is already out for testing. Additionally, I enabled Zarr v3 as a dependency (it uses threads which won't work in WASM, so perhaps it makes sense to not add it or skip any new tests that will fail due to it). |
The sister PR to accompany this PR, which adds Pyodide support for Zarr, is now also ready for review (marked above). Thank you! |
This PR adds a CI job (
emscripten-ci.yaml
) that builds WASM wheels for numcodecs for use in Pyodide-based environments using the Emscripten toolchain. This was discussed in one of the community meetings for Zarr where it was said that this change would be welcome as a pull request, and follow-up work after this is complete is planned:TODO:
Additional context
Some xrefs for reviewers and project maintainers:
scikit-image
): Build and test PyWavelets Pyodide wheels in CI PyWavelets/pywt#701 and Upload dev wheels to Anaconda.org + revamp wheels publishing workflow PyWavelets/pywt#714pandas
repository BLD, TST: Build and test Pyodide wheels forpandas
in CI pandas-dev/pandas#57896scikit-image
scikit-image/scikit-image#7350