-
-
Notifications
You must be signed in to change notification settings - Fork 330
Add Pyodide support and CI jobs for Zarr #1903
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?
Changes from 62 commits
9f5a110
29282fc
d465742
cdf0bb2
dfe0321
b100ec9
b0dddca
d227728
fdb2bef
621077a
7ae9a97
22eb6da
6836947
fe3bf27
08997ec
9bfc860
9bcb350
9985abb
7ea12ef
a6565de
85f621c
1a64255
c8cb38b
eb36d40
e3bf365
b3a5b8a
42d2792
27068e2
aff9b18
bb2c136
710195a
07e5bc9
403fbb0
24dbc77
ecea615
d919bd7
0bb7d47
fb59eba
4c6bed6
6754131
f426ed7
405d247
e93073a
d862953
cbd1d4d
3e8bdef
4ee492e
d94970e
cd3424c
5044a22
e131867
c7c22dc
e62c933
d3bcf56
e4b7379
35cecc1
ceb70c7
1ea992f
f51ddd1
0ec47b9
e1617c0
23e34bf
22e6795
da8bfc7
81f5df3
bad9920
2217455
86f8785
3230892
07b5645
ef70cbd
9212c0e
86323eb
9c6af32
090c62b
3f2d41d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
# 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.28.0a3 | ||
# 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.13 # any 3.13.x version works | ||
EMSCRIPTEN_VERSION: 4.0.9 | ||
NODE_VERSION: 22 | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
build_wasm_emscripten: | ||
name: Build and test Zarr for Pyodide | ||
runs-on: ubuntu-latest | ||
# 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 | ||
with: | ||
fetch-depth: 0 | ||
fetch-tags: true | ||
|
||
- 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 | ||
|
||
- name: Build Zarr for Pyodide | ||
run: | | ||
pyodide xbuildenv install ${{ env.PYODIDE_VERSION }} | ||
pyodide build | ||
### (Temporarily) build numcodecs as well, as we have an older version in the Pyodide distribution (v0.13.1) | ||
|
||
- name: Clone numcodecs repository | ||
uses: actions/checkout@v4 | ||
with: | ||
# See https://github.com/zarr-developers/numcodecs/pull/529 | ||
repository: agriyakhetarpal/numcodecs | ||
ref: setup-emscripten-ci | ||
path: numcodecs-wasm | ||
submodules: recursive | ||
fetch-depth: 0 | ||
fetch-tags: true | ||
|
||
# For some reason fetch-depth: 0 and fetch-tags: true aren't working... | ||
- name: Manually fetch tags for numcodecs | ||
working-directory: numcodecs-wasm | ||
run: git fetch --tags | ||
|
||
- name: Build numcodecs for WASM | ||
run: pyodide build | ||
working-directory: numcodecs-wasm | ||
env: | ||
DISABLE_NUMCODECS_AVX2: 1 | ||
DISABLE_NUMCODECS_SSE2: 1 | ||
|
||
### Back to Zarr repository to run tests | ||
|
||
- name: Run Zarr tests for Pyodide | ||
run: | | ||
# Set up Pyodide virtual environment and activate it | ||
pyodide venv .venv-pyodide | ||
source .venv-pyodide/bin/activate | ||
# Install numcodecs | ||
pip install $(ls numcodecs-wasm/dist/*.whl)"[crc32c]" | ||
# Install Zarr without dependencies until we can figure out the | ||
# numcodecs wheel versioning issue | ||
pip install dist/*.whl --no-deps | ||
pip install "packaging>=22.0" "numpy>=1.25" "typing_extensions>=4.9" "donfig>=0.8" | ||
# Install test dependencies | ||
pip install "coverage" "pytest" "pytest-asyncio" "pytest-cov" "pytest-accept" "rich" "mypy" "hypothesis" | ||
python -m pytest tests -v --cov=zarr --cov-config=pyproject.toml | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# This file only exists to not incur circular import issues | ||
# TODO: find a better location for this or keep it here | ||
|
||
from __future__ import annotations | ||
|
||
import platform | ||
import sys | ||
|
||
IS_WASM: bool = sys.platform == "emscripten" or platform.machine() in ["wasm32", "wasm64"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,7 @@ | |
from functools import cached_property | ||
from typing import TYPE_CHECKING | ||
|
||
import numcodecs | ||
from numcodecs.zstd import Zstd | ||
from packaging.version import Version | ||
|
||
from zarr.abc.codec import BytesBytesCodec | ||
from zarr.core.buffer.cpu import as_numpy_array_wrapper | ||
|
@@ -44,12 +42,12 @@ class ZstdCodec(BytesBytesCodec): | |
|
||
def __init__(self, *, level: int = 0, checksum: bool = False) -> None: | ||
# numcodecs 0.13.0 introduces the checksum attribute for the zstd codec | ||
_numcodecs_version = Version(numcodecs.__version__) | ||
if _numcodecs_version < Version("0.13.0"): | ||
raise RuntimeError( | ||
"numcodecs version >= 0.13.0 is required to use the zstd codec. " | ||
f"Version {_numcodecs_version} is currently installed." | ||
) | ||
# _numcodecs_version = Version(numcodecs.__version__) | ||
# if _numcodecs_version < Version("0.13.0"): | ||
# raise RuntimeError( | ||
# "numcodecs version >= 0.13.0 is required to use the zstd codec. " | ||
# f"Version {_numcodecs_version} is currently installed." | ||
# ) | ||
Comment on lines
+45
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Temporary change; these lines will be uncommented when we either have a solution for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about skipping tests that require newer version of numcodecs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, no, this is only to disable the version check. |
||
|
||
level_parsed = parse_zstd_level(level) | ||
checksum_parsed = parse_checksum(checksum) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,14 @@ | |
import atexit | ||
import logging | ||
import os | ||
import sys | ||
import threading | ||
from concurrent.futures import ThreadPoolExecutor, wait | ||
from typing import TYPE_CHECKING, Any, TypeVar | ||
|
||
from typing_extensions import ParamSpec | ||
|
||
from zarr._constants import IS_WASM | ||
from zarr.core.config import config | ||
|
||
if TYPE_CHECKING: | ||
|
@@ -36,6 +38,45 @@ | |
pass | ||
|
||
|
||
# TODO: not sure if there is a better place to put this function, so I've kept it here for now. | ||
def _make_shutdown_asyncgens_noop_for_pyodide() -> None: | ||
""" | ||
Patch Pyodide's WebLoop to fix interoperability with pytest-asyncio. | ||
|
||
WebLoop.shutdown_asyncgens() raises NotImplementedError, which causes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should implement it in Pyodide? If this is only problematic in testing environment, how about moving this into conftest.py? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I haven't thought about it, but yes, I think this should be helpful to put somewhere in
Yes, makes sense to me! I'll move it to Edit: I've moved the patch to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer solving it in webloop rather than just in the test code if it's possible. |
||
pytest-asyncio to issue warnings during test cleanup and potentially | ||
cause resource leaks that make tests hang. This is a bit of a | ||
hack, but it allows us to run tests that use pytest-asyncio. | ||
|
||
This is necessary because pytest-asyncio tries to clean up async generators | ||
when tearing down test event loops, but Pyodide's WebLoop doesn't support | ||
this as it integrates with the browser's event loop rather than managing | ||
its own lifecycle. | ||
""" | ||
try: | ||
if not IS_WASM and "pyodide" not in sys.modules: | ||
return | ||
|
||
import pyodide.webloop | ||
|
||
if hasattr(pyodide.webloop.WebLoop, "shutdown_asyncgens"): | ||
|
||
async def no_op_shutdown_asyncgens(self) -> None: # type: ignore[no-untyped-def] # noqa: ANN001 | ||
return | ||
|
||
pyodide.webloop.WebLoop.shutdown_asyncgens = no_op_shutdown_asyncgens | ||
logger.debug("Patched WebLoop.shutdown_asyncgens for pytest-asyncio compatibility") | ||
|
||
# If patching fails for any reason, we log it, but we won't want to crash Zarr | ||
except Exception as e: | ||
msg = f"Could not patch WebLoop for pytest compatibility: {e}" | ||
logger.debug(msg) | ||
|
||
|
||
if IS_WASM: | ||
_make_shutdown_asyncgens_noop_for_pyodide() | ||
|
||
|
||
def _get_lock() -> threading.Lock: | ||
"""Allocate or return a threading lock. | ||
|
||
|
@@ -133,6 +174,17 @@ | |
-------- | ||
>>> sync(async_function(), existing_loop) | ||
""" | ||
# WASM environments (like Pyodide) cannot start new threads, so we need to handle | ||
# coroutines differently. We integrate with the existing Pyodide WebLoop which | ||
# schedules tasks on the browser's event loop using setTimeout(): | ||
# https://developer.mozilla.org/en-US/docs/Web/API/setTimeout | ||
if IS_WASM: | ||
current_loop = asyncio.get_running_loop() | ||
return current_loop.run_until_complete(coro) | ||
agriyakhetarpal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# This code path is the original thread-based implementation | ||
# for non-WASM environments; it creates a dedicated I/O thread | ||
# with its own event loop. | ||
if loop is None: | ||
# NB: if the loop is not running *yet*, it is OK to submit work | ||
# and we will wait for it | ||
|
@@ -170,6 +222,12 @@ | |
|
||
The loop will be running on a separate thread. | ||
""" | ||
if IS_WASM: | ||
raise RuntimeError( | ||
"Thread-based event loop not available in WASM environment. " | ||
"Use zarr.api.asynchronous or ensure sync() handles WASM case." | ||
) | ||
|
||
if loop[0] is None: | ||
with _get_lock(): | ||
# repeat the check just in case the loop got filled between the | ||
|
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.
For unexplained reasons, even this doesn't help.