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

Add a Pyodide runner based on the python-execution-prototypes #893

Merged
merged 43 commits into from Feb 13, 2024

Conversation

tuzz
Copy link
Contributor

@tuzz tuzz commented Jan 18, 2024

This pull request adds experimental support for running Python programs using Pyodide. The code is based on the python-execution-prototypes and the technical analysis document. The related GitHub issue is #891.

The PyodideRunner.jsx can be used instead of the current PythonRunner.jsx (Skulpt) by provided a ?pyodide=true parameter in the URL as shown in the screenshot below:

Screenshot 2024-01-30 at 11 39 01

Notes

The PyodideRunner supports the following features:

  • Execution of Python programs
  • Importing of most of Python's standard library
  • Importing of many common Python packages
  • Importing of pure Python packages from PyPi
  • Stopping execution
  • Sending standard input to Python programs
  • Closing the standard input stream by pressing Ctrl-d
  • Visualising turtle graphics using the modified turtle package
  • Visualising pygal charts using the modified pygal package

Due to time constraints, the following tasks remain outstanding:

Copy link

tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 18, 2024
We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option.

I’ve tried to follow the migration guide here:
https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
tuzz added a commit that referenced this pull request Jan 19, 2024
… 3.11.1 to 4.0.0 (#895)

We need to be able to conditionally set headers for pyodideWorker.js in
pull request #893
but this feature isn’t supported in webpack-dev-server 3.x.x. We
therefore need to upgrade to version 4.0.0 which allows a function to be
provided in the headers option. When attempting this upgrade, I ran into
[this issue](storybookjs/storybook#22431) and
only managed to resolve it by also updating yarn to version 3.

I’ve tried to follow the migration guide here:

https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md
@tuzz tuzz temporarily deployed to previews/pyodide-runner January 19, 2024 12:34 — with GitHub Actions Inactive
Copy link

@tuzz tuzz temporarily deployed to previews/pyodide-runner January 19, 2024 13:07 — with GitHub Actions Inactive
Copy link

@tuzz tuzz temporarily deployed to previews/pyodide-runner January 30, 2024 10:42 — with GitHub Actions Inactive
Jest doesn’t support Web Workers and was erroring because the
importScripts function isn’t available. Best practice seems to be to
mock the module and returned canned responses. Currently, we don’t have
Jest tests for the experimental PyodideRunner.js so an empty
implementation is sufficient but we could change this later.

https://stackoverflow.com/questions/42567535#answer-62436219
@tuzz tuzz temporarily deployed to previews/pyodide-runner January 30, 2024 11:39 — with GitHub Actions Inactive
Copy link

@tuzz tuzz marked this pull request as ready for review January 30, 2024 11:55
@tuzz tuzz temporarily deployed to previews/pyodide-runner January 30, 2024 11:59 — with GitHub Actions Inactive
Copy link

@@ -0,0 +1,364 @@
/* eslint import/no-webpack-loader-syntax: off */
/* eslint-disable react-hooks/exhaustive-deps */
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ I'm not a fan of this rule for sure, especially with empty dependency arrays but 2/3 useEffects that are effected can have handleRun and handleStop in the array without any impact

Copy link
Contributor

Choose a reason for hiding this comment

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

raising just so I can make a TODO ticket 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was copied over from PythonRunner.jsx when I started working on PyodideRunner.jsx. I tried reinstating the eslint but it caused quite a lot of errors that will take some time to fix. Maybe we can address this later in a separate ticket as you suggest. I've addressed the other pull request comments now.

sra405
sra405 previously approved these changes Feb 12, 2024
Copy link
Contributor

@sra405 sra405 left a comment

Choose a reason for hiding this comment

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

👍 only a couple of minor things and want to get this in to build on. Great work 🥳

@sra405 sra405 temporarily deployed to previews/pyodide-runner February 12, 2024 14:42 — with GitHub Actions Inactive
Copy link

Copy link

@sra405
Copy link
Contributor

sra405 commented Feb 13, 2024

Due to time constraints, the following tasks remain outstanding:

I'll make a general ticket to list the remaining work, feel free to add any additional bits or comments 👍

@sra405 sra405 mentioned this pull request Feb 13, 2024
Copy link
Contributor

@sra405 sra405 left a comment

Choose a reason for hiding this comment

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

👍 this is ace! Let's get it in and build on top 😄

@tuzz tuzz merged commit f791ee3 into main Feb 13, 2024
8 checks passed
@tuzz tuzz deleted the pyodide-runner branch February 13, 2024 14:22
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.

Integrate the Pyodide prototype with the editor-ui project
2 participants