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

pre-install packages concurrently #164

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Carreau
Copy link

@Carreau Carreau commented Jan 30, 2025

I think that instead of doing N requests of await piplite.install, we can do one request with all the required packages.

This seem to only affect 6 packages when adding logging:

 - ssl
 - sqlite3
 - ipykernel
 - comm
 - pyodide_kernel
 - ipython

I'm assuming it won't make that much of a difference.

I think that instead of doing N requests of await piplite.install,
we can do one request with all the required packages.

This seem to only affect 6 packages when adding logging:

     - ssl
     - sqlite3
     - ipykernel
     - comm
     - pyodide_kernel
     - ipython

I'm assuming it won't make that much of a difference.
Copy link
Contributor

lite-badge 👈 Try it on ReadTheDocs

@Carreau
Copy link
Author

Carreau commented Jan 30, 2025

@bollwyvl I think wrote the original, so maybe has a reason to do so.

@bollwyvl
Copy link
Contributor

Order certainly was important at some point: there are some patches applied where certain things had to happen between two other things. This list used to be longer, but looks like we got it down to setting MPLBACKEND. I am not sure of the impact of not setting that before something else happening, and indeed, maybe that's all handled by matplotlib_inline.

On the main, the biggest win is to not play: ideally we'd further drive down the number of things handled here before thinking that parallelism is going to make anything better.

I think the addition of ssl was a poor choice: it's quite large, and (by observation) never needed for bootstrapping the package manager/kernel, which is all that should ever be required in this list. The failure mode in user content is gross, though, and I didn't see a viable way to sniff it, as some libraries make decisions on some things it provides.

sqlite is also quite chunky... and I don't think IPython's history stuff probably even really does anything useful in this context. Maybe we could patch that out.

And then, of course, as discussed a number of times, is the IPython.tests. The pyodide distribution will unvendor some tests into a sibling zip, and likely should be applied in this case.

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.

2 participants