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

Make panelite notebook installs specific to the packages that are imported #4209

Closed
MarcSkovMadsen opened this issue Dec 17, 2022 · 4 comments · Fixed by #4388
Closed

Make panelite notebook installs specific to the packages that are imported #4209

MarcSkovMadsen opened this issue Dec 17, 2022 · 4 comments · Fixed by #4388
Labels
need input from Philipp panelite type: enhancement Minor feature or improvement to an existing feature

Comments

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Dec 17, 2022

All over the place generic micropip.installs are made. An example is shown below

image

This slows down the notebook when loading. It might also lead some newbies to think that these installs are needed.

Please make the installs specific.

Additional Context

If you tell me this would be beneficial @philippjfr I would try to find the time to clean up. Right now my guess is you did it this way due to not having enough time to specialize the installs across all notebooks?

@MarcSkovMadsen MarcSkovMadsen added type: enhancement Minor feature or improvement to an existing feature need input from Philipp panelite labels Dec 17, 2022
@MarcSkovMadsen
Copy link
Collaborator Author

After looking at the panel code I believe its the code below that has to be improved such that the imports depends on the notebook

image

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Feb 4, 2023

My recommendation would be to make this specific by including it in the notebook.

Currently it takes 28 secs for me to piplite.install the Notifications.ipynb notebook. Something that makes the notebook almost not useful. And if you have a slow internet connection or device its even worse.

image

I would recommend making the notebooks specific. Something like

image

This takes less than 3 secs on my laptop, which is very usable.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Feb 4, 2023

Would you accept a PR with a change like this across notebooks @philippjfr ? I would also clean up the existing code that adds the big, generel piplite.install.

UPDATE: I DON'T THINK THIS IS A GOOD IDEA. I WOULD RECOMMEND DEFINING A LIST SPECIFIC LIST OF REQUIREMENTS THAT WE CAN PREPEND WHEN BUILDING PANELITE.

@philippjfr
Copy link
Member

If you have a good solution for this problem i would to see a PR yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need input from Philipp panelite type: enhancement Minor feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants