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

feat: JupyterLite integration #253

Merged
merged 11 commits into from
Sep 11, 2024
Merged

feat: JupyterLite integration #253

merged 11 commits into from
Sep 11, 2024

Conversation

Saransh-cpp
Copy link
Member

Closes #250

@dpshelio the preview for this is available on my fork - https://saransh-cpp.github.io/rsd-engineeringcourse.

Going to https://saransh-cpp.github.io/rsd-engineeringcourse/jupyter-lite will redirect the users to -

image

I couldn't embed the page within the original website + all the examples having these embedded kernels are out of date/have switched to the non-embedded format (like the one in this PR).

For adding a button adjacent to "Notebook download" button, should I make a PR to the theme repository, making changes somewhere around here -

https://github.com/dpshelio/indigo-jekyll/blob/7d12d6e04f79bbb97aa59f416746409391d0cbaf/_includes/base.html#L153

Copy link
Member

@dpshelio dpshelio left a comment

Choose a reason for hiding this comment

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

Awesome! It works almost perfectly!
Some things to improve:

  • keep directories structure
  • geopy doesn't work, though it's part of the requirements. Do you know what cold be producing that?
  • I'd like to get a menu next to download notebook / like run notebook or jupyterlite icon, but that is in the theme repository. We can do it after we fix the other things.

.github/workflows/build_site.yml Outdated Show resolved Hide resolved
.github/workflows/build_site.yml Outdated Show resolved Hide resolved
@Saransh-cpp
Copy link
Member Author

geopy doesn't work, though it's part of the requirements. Do you know what cold be producing that?

Hmm, this is weird, looking into it.

@Saransh-cpp
Copy link
Member Author

geopy doesn't work, though it's part of the requirements. Do you know what cold be producing that?

I looked into this, and turns out jupyterlite does not really read dependencies from the existing environment. We can either Ship additional Pyodide wheels at build time or add a %pip install geopy cell at top. I talked to one of the pyodide developers and shipping additional wheels would just be an overkill for this. Also, students will have to install geopy if they download the notebook, so I think adding an installation statement in the notebooks should be good ✅

.gitignore Outdated Show resolved Hide resolved
@agriyakhetarpal
Copy link

Hi there, if either of you would like to add and maintain geopy in the Pyodide repository with a few associated tests, please feel welcome to do so. Having a maintainers for package recipes lets us know who to ping if there's a problem, since they as experienced users of said package(s) can help fix it when we bump the versions from time to time. :)

@Saransh-cpp
Copy link
Member Author

Saransh-cpp commented Sep 9, 2024

Thanks for the suggestion! One thing that I was looking at (after you shared it) was Creating a Pyodide package. The guide says -

If you wish to use a package in Pyodide that is not already included in the packages folder, first you need to determine whether it is necessary to package it for Pyodide. Ideally, you should start this process with package dependencies.

Most pure Python packages can be installed directly from PyPI with micropip.install() if they have a pure Python wheel. Check if this is the case by trying micropip.install("package-name").

Given that geopy is a pure Python package, should it be added as a "package" in Pyodide?

@agriyakhetarpal
Copy link

Yes, to avoid the usual answer of "it depends": I think it's sometimes good to add pure Python packages to Pyodide, and many of them exist – especially those in Python's scientific computing ecosystem. For example, Zarr is a pure Python package, but it relies on async programming functionality, which means that it's good to include it and test it from time to time. I see that geopy depends on geographiclib, which from a quick search is a C++-based package, so it could be worth adding (haven't looked at that package in detail, though). Additionally, geopy offers a few extra features through optional dependencies, so if you're using those in your course material, they could benefit from some end-to-end testing ;)

Additing it would constrain you to use the version of it available with the Pyodide version, though, unless you disable installing it and ask for the one to be used from PyPI explicitly. If that's a bother and you would like to keep using the latest versions available on PyPI for your convenience, then you may skip doing this. Nevertheless, it would be required to add it if geopy were to adopt a C/C++ extension someday and switch away from being a pure Python package.

@Saransh-cpp
Copy link
Member Author

Thanks for the details! geopy, in fact, is not working out of the box in jupyterlite. I have created an issue in their repository, but it looks like the development there has stalled? I'll keep a look if I get any replies from them.

@Saransh-cpp
Copy link
Member Author

Copying from geopy/geopy#578

... the issue is originating from pyodide's limitations and not from geopy. The other requests in our course repo fail in pyodide as well, confirming that geopy is not the issue here ...

Added comments to mark bash and requests failure on the browser.

Copy link
Member

@dpshelio dpshelio left a comment

Choose a reason for hiding this comment

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

🚀 Thank you for all this!!

@dpshelio dpshelio enabled auto-merge September 11, 2024 07:30
@dpshelio dpshelio merged commit c9ab2db into main Sep 11, 2024
5 checks passed
@Saransh-cpp Saransh-cpp deleted the saransh/jupyterlite branch September 11, 2024 09:07
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.

Add option to run the notebooks from the website
3 participants