CI Add Pyodide wheel build #26374
CI Add Pyodide wheel build #26374
Conversation
Not really, when you fix the Pyodide version (as already done), you get a fixed version of the packages (e.g. numpy, scipy, etc ...). The main reason I used Azure is that I thought Pyodide was similar to PyPy and nogil, an exotic platform that we want to test during the the nightly build and on a explicit commit message. I was also planning to use the python script to create an issue when the Pyodide nightly build fails.
Can you explain a bit why? I guess the |
There was a problem hiding this comment.
Can you explain a bit why?
Two reasons:
- Purely logistical. We currently have 60 runners on GitHub Actions and 30 runners on Azure.
- We are going to need a new yaml file to configure Pyodide anyways, so maybe move to GitHub Actions.
In any case, I am okay with including this in the Azure CI.
+1 for at least checking that the import works + a note that explains that running the full test suite will come once a stable release of pyodide makes it possible. |
|
No opinion on Azure vs GitHub Actions. I don't think worker usage is a problem because it is not trigger on the PR, only on scheduled builds + manual trigger. |
There was a problem hiding this comment.
Looks good to me once https://github.com/scikit-learn/scikit-learn/pull/26374/files#r1195164229 is agreed upon. We can later refactor and move to github actions shall the need arise.
Note: there is a warning in the build log:
WARNING: Location 'file:/home/vsts/work/1/s/.pyodide-xbuildenv/xbuildenv/pyodide-root/pypa_index/pip/' is ignored: it is neither a file nor a directory.
not sure if this is a real problem or not.
Not sure either, I tried to reproduce locally and I didn't manage to get the warning somehow ... I would say everything seems to work fine so probably good enough? |
|
Great, the Pyodide nightly build worked fine: https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=55195&view=logs&j=6fac3219-cc32-5595-eb73-7f086a643b12 |
What does this implement/fix? Explain your changes.
This adds a CI build that build a Pyodide wheel:
[pyodide]is in the commit messageI am not planning to run the scikit-learn tests in this PR. This is already useful to make sure the Pyodide wheel builds fine. There was at least one issue in the past where some changes broke the Pyodide build, see #25831.
Right now the scikit-learn test suite pass on the Pyodide development version (but not on the latest released version namely 0.23.2 at the time of writing) so probably I will wait for the next Pyodide release before running tests. I could also add something simple like making sure that
import sklearnworks or run the test suite for some submodules that are known to work, let me know!For more details about scikit-learn test suite status in Pyodide see https://github.com/lesteve/scikit-learn-tests-pyodide.
Edit: the Pyodide build seems to work fine.