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

Allow setting pyodide version #67

Merged
merged 4 commits into from
Dec 5, 2024
Merged

Conversation

ahuang11
Copy link
Contributor

@ahuang11 ahuang11 commented Dec 5, 2024

Closes #66

I probably am missing a few pieces.

@pawamoy
Copy link
Owner

pawamoy commented Dec 5, 2024

Awesome, thank you for the super fast PR!

Looking at the existing code, I think version will end up in the extra dict. Try to pop it from there. Other than that, can you use .format instead of %? You probably used it for consistency 👍 it was easier to use % in the template variable because there are curly braces in there 😄

@pawamoy
Copy link
Owner

pawamoy commented Dec 5, 2024

Don't pay attention to CI failures, they're unrelated.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Dec 5, 2024

it was easier to use % in the template variable because there are curly braces in there 😄

Aha yeah, I was wondering why the old school % was used over format, but that's a good point!

Copy link
Owner

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

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

Looks perfect, thanks! Can you just confirm it actually works (since Pyodide was upgraded), and whether it fixes #66? 😄

@pawamoy
Copy link
Owner

pawamoy commented Dec 5, 2024

Oh we'll need some docs, though I can take care of that if you're unsure about it.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Dec 5, 2024

Yeah if you can handle docs that'd be great and let me test and report back soon...

@pawamoy
Copy link
Owner

pawamoy commented Dec 5, 2024

OK looks like it's working well, and the version upgrade also fixes #66. Thank you so much!

@pawamoy pawamoy merged commit 912c8c7 into pawamoy:main Dec 5, 2024
2 of 26 checks passed
@ahuang11
Copy link
Contributor Author

ahuang11 commented Dec 5, 2024

Thanks! It seems to be working past the installation.

However, I don't think it's rendering the Panel widgets. I might have to research a bit on https://github.com/holoviz-dev/nbsite/tree/main/nbsite/pyodide and see what's done differently.

image

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.

feature: Bumping Pyodide version or allow specifying pyodide version to use
2 participants