scikit-hep / uproot5

ROOT I/O in pure Python and NumPy.
https://uproot.readthedocs.io
BSD 3-Clause "New" or "Revised" License
239 stars 76 forks source link

ci: add job to test with Emscripten #1272

Closed ariostas closed 2 months ago

ariostas commented 3 months ago

This PR resumes the work started in #1026. The package is installed and used with pyodide, just how it was done in #1026. I considered switching to using emscripten-forge and xeus-python since there are some significant advantages for deployment, but currently it is much easier to set up these tests with pyodide.

The main difficulty with the tests are network requests and thread spawning. Network issues don't end up being as bad for deployments since often files are hosted on the same server/website (and there's ways to get around CORS protections), so I'll see if I can get some very simple http test working. As for thread spawning, it's more of an issue since there are multiple functions that try to spawn threads without being explicit that they are doing so. Currently WASM doesn't support multiple threads, so various things don't work.

I hand-picked a few test files that work, but probably the best approach will be to introduce a new pytest marker to indicate which tests should be attempted. And maybe it would be good to decide which subset of the functionality should be available in WASM and write some simple tests for it to make it more explicit.

ariostas commented 2 months ago

I think this is starting to be in good shape, so it would be nice to get some feedback.

There are quite a few things that changed since my initial comment.

ariostas commented 2 months ago

Thank you Jim, that's a great suggestion.

I introduced uproot._util.wasm which is set to True when the platform is Emscripten or WASI. In general, I had to think a bit whether to name things with WASM, Emscripten, or Pyodide suffixes, but I think now it's reasonable.