-
-
Notifications
You must be signed in to change notification settings - Fork 287
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
[WIP]: Add Pyodide support and CI jobs for Zarr #1903
Draft
agriyakhetarpal
wants to merge
24
commits into
zarr-developers:main
Choose a base branch
from
agriyakhetarpal:emscripten
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
9f5a110
Add CI job to test out-of-tree Pyodide builds
agriyakhetarpal 29282fc
Add `[msgpack]` dependency for `numcodecs`
agriyakhetarpal d465742
Bump to Pyodide 0.26.0, update comments
agriyakhetarpal cdf0bb2
Try to run tests without async
agriyakhetarpal dfe0321
Move shared file to rootdir, outside v2 and v3
agriyakhetarpal b100ec9
Move `fasteners` import inside ThreadSynchronizer
agriyakhetarpal b0dddca
Make the tests directory importable, fix `_shared`
agriyakhetarpal d227728
Import list of greetings from `numcodecs`
agriyakhetarpal fdb2bef
Skip some tests that use threading
agriyakhetarpal 621077a
Skip some tests that use `fcntl`
agriyakhetarpal 7ae9a97
Skip tests that require `dbm`
agriyakhetarpal 22eb6da
Move `IS_WASM` logic to internal `zarr` API
agriyakhetarpal 6836947
Skip a few tests trying to import `multiprocessing`
agriyakhetarpal fe3bf27
Skip tests that use async and threading code
agriyakhetarpal 08997ec
Improve `asyncio_tests_wrapper`, fix test imports
agriyakhetarpal 9bfc860
Skip entire `test_codecs.py` file
agriyakhetarpal 9bcb350
Skip yet another test that requires threads
agriyakhetarpal 9985abb
xfail test where array's fill values are different
agriyakhetarpal 7ea12ef
xfail test because Emscripten FS
agriyakhetarpal a6565de
Skip last test that tries to run threads
agriyakhetarpal 85f621c
Another test that tries to run threads
agriyakhetarpal 1a64255
xfail another array's differing `fill_values` test
agriyakhetarpal c8cb38b
Skip entire sync file under WASM, no threading
agriyakhetarpal eb36d40
Restore pytest config options, remove when needed
agriyakhetarpal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Attributed to NumPy https://github.com/numpy/numpy/pull/25894 | ||
# https://github.com/numpy/numpy/blob/d2d2c25fa81b47810f5cbd85ea6485eb3a3ffec3/.github/workflows/emscripten.yml | ||
|
||
name: Pyodide wheel | ||
|
||
on: | ||
# TODO: refine after this is ready to merge | ||
[push, pull_request, workflow_dispatch] | ||
|
||
env: | ||
FORCE_COLOR: 3 | ||
PYODIDE_VERSION: 0.26.0 | ||
# PYTHON_VERSION and EMSCRIPTEN_VERSION are determined by PYODIDE_VERSION. | ||
# The appropriate versions can be found in the Pyodide repodata.json | ||
# "info" field, or in Makefile.envs: | ||
# https://github.com/pyodide/pyodide/blob/main/Makefile.envs#L2 | ||
PYTHON_VERSION: 3.12.1 | ||
EMSCRIPTEN_VERSION: 3.1.58 | ||
NODE_VERSION: 18 | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
permissions: | ||
contents: read # to fetch code (actions/checkout) | ||
|
||
jobs: | ||
build_wasm_emscripten: | ||
name: Build and test Zarr for Pyodide | ||
runs-on: ubuntu-22.04 | ||
# To enable this workflow on a fork, comment out: | ||
# FIXME: uncomment after this is ready to merge | ||
# if: github.repository == 'zarr-developers/zarr-python' | ||
steps: | ||
- name: Checkout Zarr repository | ||
uses: actions/checkout@v4 | ||
|
||
- name: Set up Python ${{ env.PYTHON_VERSION }} | ||
id: setup-python | ||
uses: actions/setup-python@v5 | ||
with: | ||
python-version: ${{ env.PYTHON_VERSION }} | ||
|
||
- name: Set up Emscripten toolchain | ||
uses: mymindstorm/setup-emsdk@v14 | ||
with: | ||
version: ${{ env.EMSCRIPTEN_VERSION }} | ||
actions-cache-folder: emsdk-cache | ||
|
||
- name: Set up Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: ${{ env.NODE_VERSION }} | ||
|
||
- name: Install pyodide-build | ||
run: python -m pip install "pyodide-build==${{ env.PYODIDE_VERSION }}" | ||
|
||
- name: Build Zarr for Pyodide | ||
run: | | ||
pyodide build | ||
|
||
- name: Run Zarr tests for Pyodide | ||
run: | | ||
# Avoid missing asyncio plugin error from pytest, unavailable in Pyodide | ||
if grep -q 'asyncio_mode = "auto"' "pyproject.toml"; then sed '/asyncio_mode = "auto"/d' "pyproject.toml" > temp && mv temp "pyproject.toml"; fi | ||
pyodide venv .venv-pyodide | ||
source .venv-pyodide/bin/activate | ||
python -m pip install dist/*.whl | ||
python -m pip install pytest pytest-cov | ||
python -m pytest -v --cov=zarr --cov-config=pyproject.toml | ||
|
||
- name: Upload Pyodide wheel artifact for debugging | ||
# FIXME: Remove after this is ready to merge | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: zarr-pyodide-wheel | ||
path: dist/*.whl |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be a problem for moving this PR forward. The entire design of V3 depends on being able to execute async. If threading and async are a show stopper, I think we should evaluate whether this can work another way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I think about half of the test suite is being skipped because of these tests under v3. Both threading and async code are probably not a priority at this time for Pyodide (perhaps @hoodmane will have more insights).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the latest run on my fork:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume 99% of the passing tests are coming from the
v2
test suite. I wouldn't put much weight on that. We needtests/v3
to be passing for this to work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. If there is a way to run asynchronous tests in a manner that is not asynchronous, then there might be a way forward, but if Zarr's underlying functionality is going to rely on this, I am not sure if there is a solution. The first way would be to get
pytest-asyncio
both packaged and running under Pyodide.Edit: the related issue is pyodide/pyodide#2221.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is at this point possible to run async tests in Pyodide using stack switching. Just have to implement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pushing this forward @agriyakhetarpal.