Skip to content
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

MNT Update to Pyodide 0.24 #27405

Merged
merged 3 commits into from Sep 19, 2023
Merged

Conversation

betatim
Copy link
Member

@betatim betatim commented Sep 18, 2023

Reference Issues/PRs

closes ##26763

What does this implement/fix? Explain your changes.

This changes the version of pyodide used to 0.24.0.

Any other comments?

I grepped the repo for 0.23.4 (previous version) and pyodide to find all the places where the version needs changing. These are the only places I found. Does someone know if there are more?

@github-actions
Copy link

github-actions bot commented Sep 18, 2023

✔️ Linting Passed

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

Generated for commit: 1bbc4a2. Link to the linter CI: here

@adrinjalali
Copy link
Member

duplicate of #27346 ?

@betatim
Copy link
Member Author

betatim commented Sep 18, 2023

Kind of, I think #27346 tries to do more than this PR.

@lesteve
Copy link
Member

lesteve commented Sep 19, 2023

Thanks for the PR, a few comments:

  • jupyter-lite.json sets the Pyodide version that is used (hence also the scikit-learn version, Pyodide 0.24 has scikit-learn 1.3.0) when you click on the JupyterLite button in a gallery example
  • the one in azure-pipelines.yml is for building a scikit-learn wheel. Right now there is a bug in pyodide-build 0.24.0 see Fix replacing local include paths pyodide/pyodide#4136, so I reverted this part which would make the Pyodide build fail. The Pyodide build is similar to the scipy-dev one and gets run in PRs only if you add [pyodide] in the commit message
  • CI Run test suite inside Pyodide #27346 aims at running the scikit-learn tests inside Pyodide which should pass except a few tests to xfail because of Pyodide limitations

@betatim
Copy link
Member Author

betatim commented Sep 19, 2023

  • jupyter-lite.json sets the Pyodide version that is used (hence also the scikit-learn version, Pyodide 0.24 has scikit-learn 1.3.0) when you click on the JupyterLite button in a gallery example

It is pretty cool (and makes sense :D) that there is just that one thing to change to get a different version of pyodide for our built docs. Almost too easy.

Should we merge this PR then to get the example gallery working with 1.3.0?

@glemaitre if we merge this, do I need to do something to get it included in the 1.3.1 release? (this time maybe pinging you is enough, but in the future is there something I can do?)

@lesteve
Copy link
Member

lesteve commented Sep 19, 2023

Let me test it locally before merging (due to CircleCI limitations the JupyterLite button does not work in CircleCI artifacts 😢)

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming Loic's local build yield the expected result, LGTM as well.

@lesteve
Copy link
Member

lesteve commented Sep 19, 2023

So trying to build locally and test the JupyterLite, it seems there are some issues that need to be looked at.

The first one is that Pyodide 0.24 has been fixed in jupyterlite-pyodide-kernel 0.1.2 (see jupyterlite/pyodide-kernel#62). Trying to update the lock file I then get issues during the sphinx doc build which I don't understand yet ...

@lesteve lesteve enabled auto-merge (squash) September 19, 2023 11:49
@lesteve
Copy link
Member

lesteve commented Sep 19, 2023

I pushed a lock file update for the doc build and enabled auto-merge.

The local issues I was noticing were due to something weird in my local setup ...

@lesteve lesteve merged commit 539b529 into scikit-learn:main Sep 19, 2023
25 checks passed
@ogrisel
Copy link
Member

ogrisel commented Sep 19, 2023

This needs to be backported to 1.3.X probably.

@ogrisel ogrisel added this to the 1.3.1 milestone Sep 19, 2023
@lesteve
Copy link
Member

lesteve commented Sep 19, 2023

FYI, I double-checked and it seems like this works on the scikit-learn dev website: https://scikit-learn.org/dev/lite/lab/?path=auto_examples/release_highlights/plot_release_highlights_1_3_0.ipynb

scikit-learn version is indeed 1.3.0.

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 19, 2023
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
@betatim betatim deleted the update-pyodide-0.24 branch September 20, 2023 06:03
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants