CI Run test suite inside Pyodide by lesteve · Pull Request #27346 · scikit-learn/scikit-learn

lesteve · 2023-09-12T10:12:28Z

With Pyodide 0.24, the scikit-learn test suite should pass (except a few tests that can be xfailed due to Pyodide limitations).

…ikit-learn#4136

github-actions · 2023-09-15T13:02:47Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 4d911d6. Link to the linter CI: here}

…nto run-tests-in-pyodide

lesteve · 2023-09-20T07:51:17Z

I am marking this PR as ready for review. Here are a few comments:

pyodide build has a bug in 0.24, this is fixed in Pyodide main but not in a release yet see Fix replacing local include paths pyodide/pyodide#4136. For now I am applying a patch manually to be able to build a Pyodide wheel
there is a genuine test failure in sklearn/utils/tests/test_sparsefuncs.py test_min_max with sparse arrays and int64 index dtype that has been introduced recently by one of the sparse array PRs that add tests for sparse arrays (see build log). For now I have marked this test as xfailed and I propose to tackle this in a separate PR.
codecov failure is due to the fact that Pyodide does not upload its coverage. I think we can live with this for now.

ogrisel

LGTM besides the following suggestions:

sklearn/experimental/tests/test_enable_hist_gradient_boosting.py

sklearn/utils/tests/test_sparsefuncs.py

ogrisel · 2023-09-26T10:23:53Z

build_tools/azure/install_pyodide.sh

@@ -14,9 +14,3 @@ pip install pyodide-build==$PYODIDE_VERSION pyodide-cli
 pyodide build


Would it be possible to speed-up the build by configuring ccache as we do for the other linux and macos builds?

Maybe in a separate PR?

I tried setting ccache with emscripten locally and for some reason, I get a poor cache hit rate of ~30%. Locally building without the cache takes ~4 minutes and with the cache ~3 minutes. I am not too sure how this would translate in the CI.

Right now, building the wheel is ~10 minutes and running the tests ~25 minutes.

ogrisel · 2023-09-26T10:26:58Z

build_tools/azure/install_pyodide.sh

-pyodide venv pyodide-venv
-source pyodide-venv/bin/activate
-
-pip install dist/*.whl


Why isn't this step no longer required? Is it because we this is replaced by npm install pyodide@$PYODIDE_VERSION?

If so, why not call npm install as part of the install_pyodide.sh script?

The Pyodide venv does not work with scipy.linalg for now (pyodide/pyodide#3865), so you need to go the js wrapper way. Previously we were only importing scikit-learn and avoiding the issue.

Can we add a comment here or above npm install pyodide@$PYODIDE_VERSION to explain the situation?

sklearn/_loss/tests/test_loss.py

sklearn/utils/tests/test_sparsefuncs.py

thomasjpfan · 2023-09-27T11:47:58Z

build_tools/azure/install_pyodide.sh

-pyodide venv pyodide-venv
-source pyodide-venv/bin/activate
-
-pip install dist/*.whl


Can we add a comment here or above npm install pyodide@$PYODIDE_VERSION to explain the situation?

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

…kit-learn into run-tests-in-pyodide

thomasjpfan

LGTM

lesteve · 2023-09-29T07:35:12Z

Nice to see this one merged!

CI Run test suite inside Pyodide (scikit-learn#27346)

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

lesteve added 2 commits September 12, 2023 12:10

Run test suite inside Pyodide

b810a95

[pyodide]

5ccc1e3

lesteve marked this pull request as draft September 12, 2023 11:34

lesteve mentioned this pull request Sep 12, 2023

Compilation error with out-of-tree builds with 0.24.0a1: LONG_BIT definition appears wrong for platform pyodide/pyodide#4118

Closed

[pyodide build] Hack to get pyodide build to work based on Pyodide sc…

82da466

…ikit-learn#4136

lesteve added 6 commits September 15, 2023 15:06

[pyodide] [azure parallel]

b07cab0

[pyodide] [azure parallel] fix

fb1d270

[pyodide] [azure parallel] fix

845c24a

[pyodide] [azure parallel] fix

6edef7c

[pyodide] [azure parallel] first attempt at running the test suite

9835969

[pyodide] [azure parallel] fix

b24a4f9

adrinjalali mentioned this pull request Sep 18, 2023

MNT Update to Pyodide 0.24 #27405

Merged

lesteve added 4 commits September 19, 2023 18:07

xfail tests

be87e2b

[pyodide] trigger CI

4434223

Merge branch 'main' of https://github.com/scikit-learn/scikit-learn i…

fccfd15

…nto run-tests-in-pyodide

[pyodide] trigger CI

134be59

lesteve added No Changelog Needed Build / CI labels Sep 19, 2023

lesteve added 3 commits September 20, 2023 08:02

Temporary xfailing test before fixing it in main in a separate PR

a106d8a

[pyodide] trigger CI

a0761b0

[pyodide] xfail the right test

cae693b

lesteve marked this pull request as ready for review September 20, 2023 07:42

lesteve changed the title ~~Run test suite inside Pyodide~~ CI Run test suite inside Pyodide Sep 20, 2023

glemaitre self-requested a review September 25, 2023 15:41

[pyodide] use Pyodide 0.24.1

64ba556

ogrisel approved these changes Sep 26, 2023

View reviewed changes

sklearn/experimental/tests/test_enable_hist_gradient_boosting.py Show resolved Hide resolved

sklearn/utils/tests/test_sparsefuncs.py Outdated Show resolved Hide resolved

lesteve mentioned this pull request Sep 26, 2023

Error inside Pyodide with sklearn.utils.sparsefuncs on scipy sparse arrays and int64 indices #27470

Closed

[pyodide] reference issue for temporarily skipped test

4b506f4

ogrisel reviewed Sep 26, 2023

View reviewed changes

ogrisel mentioned this pull request Sep 26, 2023

MAINT downcast indices dtype when converting sparse arrays #27372

Merged

thomasjpfan reviewed Sep 27, 2023

View reviewed changes

lesteve and others added 6 commits September 27, 2023 18:07

Update sklearn/_loss/tests/test_loss.py

1e23d6d

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/utils/tests/test_sparsefuncs.py

b2490a2

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

[pyodide] Add comments

7ad26f9

Merge branch 'run-tests-in-pyodide' of https://github.com/lesteve/sci…

422a76a

…kit-learn into run-tests-in-pyodide

[pyodide] lint

512ee2a

[pyodide] fix

4d911d6

lesteve mentioned this pull request Sep 28, 2023

MAINT Use scipy sparse nanmin/nanmax for scipy>=1.11 #27492

Merged

thomasjpfan approved these changes Sep 28, 2023

View reviewed changes

thomasjpfan merged commit 751ccc0 into scikit-learn:main Sep 28, 2023

lesteve deleted the run-tests-in-pyodide branch September 29, 2023 07:33

glemaitre removed their request for review September 29, 2023 20:23

KartikeyBartwal added a commit to KartikeyBartwal/scikit-learn that referenced this pull request Sep 30, 2023

Merge pull request #2 from KartikeyBartwal/next2

6ebebae

CI Run test suite inside Pyodide (scikit-learn#27346)

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023

CI Run test suite inside Pyodide (scikit-learn#27346)

080acfc

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

lesteve mentioned this pull request Dec 13, 2023

HistGradientBoosting pickle portability between 64bit and 32bit arch #27952

Closed

		@@ -14,9 +14,3 @@ pip install pyodide-build==$PYODIDE_VERSION pyodide-cli
		pyodide build

Uh oh!

Conversation

lesteve commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

lesteve commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ogrisel Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

lesteve Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

lesteve Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasjpfan Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thomasjpfan Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

lesteve commented Sep 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lesteve commented Sep 12, 2023 •

edited

Loading

github-actions bot commented Sep 15, 2023 •

edited

Loading

lesteve commented Sep 20, 2023 •

edited

Loading

lesteve Sep 26, 2023 •

edited

Loading

lesteve Sep 26, 2023 •

edited

Loading