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.
CI Move Pyodide CI from Azure to GitHub Actions #29791
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
CI Move Pyodide CI from Azure to GitHub Actions #29791
Changes from all commits
22aa88f
37884ac
d9d43fa
776e2c9
85d644d
527c151
5ba18f0
d04b38e
d6708a6
2760c38
16267d7
961a9d4
ee3228e
383bf7e
7efd95d
cfae5ff
3ae370f
f16a501
382c8ba
049ccca
b398144
3af4ea2
4a45a97
745f6f9
0675412
a9d7e6c
86247ad
5b4d043
8acf67e
ecd3104
17f1c64
6759395
9a617e1
8d96f2a
e02c0e2
a62074c
bf8ed31
24388f1
ffc0cb8
16b0761
033c488
b4b6ed7
345bbed
04d850c
99104e7
9f78d4d
d7b3254
c434599
93ef796
6e8a6cb
03eb460
e93a405
f122250
e4091ec
6058521
c243c16
f19cca9
bf0c3e5
1e41a61
b6b3e09
5c9b11c
2fd9b50
6802778
09ded37
74b7344
4831d6d
8f81fa7
1db9ebd
a7bb1e6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 would be nice to upload the Pyodide wheel to anaconda.org when the tests pass, like numpy does. I guess this can be done in a separate PR.
The middle-term goal would be to use the Pyodide wheel on anaconda.org inside our doc JupyterLite.
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, I left it for a follow-up PR, indeed. On being able to use the Pyodide wheel, there are two approaches I have in mind:
pip
or cURL) for the nightly wheels (requires Add a--index-urls
CLI flag for pip-install magics jupyterlite/pyodide-kernel#166)scikit-learn
wheel to install which will be synced with the docs version. This is the approach I've taken in Add JupyterLite-powered interactive galleries to thescikit-image
documentation scikit-image/scikit-image#7644 that you reviewed some time back ;) (I'd be grateful for another look there!)Which one of these would you prefer?
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.
My preference would be the following for now because I feel this is the simplest option (we can always change our mind later):
notebook_modification_function
)One of my concern for your approach in scikit-image is the size of the wheel (~12M for scikit-learn Pyodide wheel). On each commit of the repo, we will push an updated wheel and the size of the scikit-learn.github.io repo grows quite quickly. To be honest, maybe the size of the Pyodide wheel is actually quite small compared to the size of the website (~300MB) ...
I'll try to have a closer look although I can't make any promises 😅
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.
Ah, I see – the increased size of the docs repository would make it quite big (I thought new builds are being overwritten on the gh-pages branch). Also, I see that the size of the zipped docs is monotonically increasing with each version for recent versions: https://scikit-learn.org/dev/versions.html, so while it it's okay for archives, it's indeed nicer if a GitHub repository does not include any more binary files than it needs to.
I'm happy to implement the notebook modifications you suggest once this PR is merged!
This file was deleted.
This file was deleted.
This file was deleted.
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.
Here,
cache
was a spurious argument that was unused throughout the file. It broke the Pyodide tests and returned an error upon collection because the interpreter assumed that this was a fixture.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 weird
__import__
is not supported in Pyodide, I just tried__import__('joblib')
in Pyodide console and this fails.What I am guessing happens is that this test somehow works because
pkg.util_walkpackages
populatessys.modules
and__import__("bla")
works inside Pyodide if"bla"
is already insys.modules
. Oh well 🤷 ...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'm not sure why that's the case. I tried it just now – both in JupyterLite and in the Pyodide (stable) console and I was able to
__import__("joblib")
to get<module 'joblib' from '/lib/python3.12/site-packag es/joblib/__init__.py'>
. I did need to installjoblib
withmicropip
, of course. Could you please check this again, just in case I might be missing something?Either way, it's nice that the test now works – I hope!
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 think
micropip.install('joblib')
makessys.modules['joblib']
exist and so__import__
work.In any case this is not really important since the test appears to pass now 🤷.