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

[VWIP] Python overhaul #75

Merged
merged 20 commits into from
May 10, 2020

Conversation

teonbrooks
Copy link
Collaborator

This is the initial attempts to gut out the dependency on a local Python kernel and the communication between the app and kernel. Pyodide offers a full working kernel implemented in WebAssembly. This PR attempts to identify pain points, necessary patches for upstream dependencies, and testing ground for change.

@teonbrooks
Copy link
Collaborator Author

you can follow along with the wishlist of things for brainwaves x pyodide here.

@jdpigeon
Copy link
Contributor

Does this PR deprecate #74?

@teonbrooks
Copy link
Collaborator Author

yes. closes #74.

#74 was done to show pyodide working without breaking anything.

Copy link
Collaborator Author

@teonbrooks teonbrooks left a comment

Choose a reason for hiding this comment

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

just moved some files that were left in the jupyter folder. forgot to move them with the other ones. let me know when you get to the plotting part with matplotlib or if that will be in a separate pr. i have some notes on how to override some implicit behavior and have it just target a dom element

app/epics/pyodideEpics.js Outdated Show resolved Hide resolved
@jdpigeon
Copy link
Contributor

I think we'll make this PR the place for a complete Jupyter replacement. Hopefully can find some time to get to the matplotlib integration step soon.

@teonbrooks
Copy link
Collaborator Author

cool. getting a matplotlib plot to work should be straightforward. there is just a little bit we would need to tweak but I have it documented.
getting the epochs viewer will be a bit more work. i think it would be a matter of getting some event listeners in place.
happy to hack on this with you, should be around town for a bit

@teonbrooks
Copy link
Collaborator Author

@jdpigeon should we make a to-do list of things we have left to address here?

@teonbrooks
Copy link
Collaborator Author

teonbrooks commented Aug 3, 2019

At this point we have been able to handle the follow:

  • go from loading raw csv -> making raw object -> filtering -> epochs

We have the following left:

  • plot the interactive epoch viewer (discussed with dano, and we are planning to remove interactive plot for now)
  • connect the epochs to the analyze screen (add some general EEG thresholding for rejection)
  • plotting the condition differences

Some things to keep in mind:

  • the loading of pyodide is a bit slow
    • this is blocking the main thread, which it should not be
  • the import might be slowed down with the loading of Pandas in addition to SciPy
  • we might want to mount the virtual filesystem to the real filesystem so we can have direct read and write from the app via Pyodide (currently restricted to the js side)

@teonbrooks
Copy link
Collaborator Author

closes #43

@teonbrooks
Copy link
Collaborator Author

hey @jdpigeon, could you rebase this branch to the latest master and I will then cherry-pick some of the commits from my local branch to get it all up-to-date

teonbrooks and others added 14 commits April 25, 2020 20:10
this adds the original pyodide.js with no mods. doesn't work out of the box. gitignore src files for building. the good place if we need to remove from git history if better solution come for importing
the objective is to clean up and organize the python calls.
- pyimport contains all the python modules to be used
- utils is for all the function definitions
- cells are independent commands to be made (maybe rename to commands)

pipes.js and function.js were responsible for managing the state of the python kernel. they won't be needed in this new paradigm
to make it clear that python is being served using WebAssembly and not the jupyter, changing all the references
@teonbrooks
Copy link
Collaborator Author

stumbled upon this, which might be useful:
https://pyodide.readthedocs.io/en/stable/using_pyodide_from_webworker.html
https://github.com/iodide-project/pyodide/blob/master/src/webworker.js

since the data processing is happening in python world and js only need images, maybe those can be passed with the onMessage. it will take python off the main thread and speed up the app.

@jdpigeon
Copy link
Contributor

stumbled upon this, which might be useful:
https://pyodide.readthedocs.io/en/stable/using_pyodide_from_webworker.html
https://github.com/iodide-project/pyodide/blob/master/src/webworker.js

since the data processing is happening in python world and js only need images, maybe those can be passed with the onMessage. it will take python off the main thread and speed up the app.

That sounds brilliant. Thanks Teon

@jdpigeon jdpigeon changed the base branch from master to dano-pyodide May 10, 2020 04:47
@jdpigeon jdpigeon merged commit b6a87aa into makebrainwaves:dano-pyodide May 10, 2020
@jdpigeon
Copy link
Contributor

Moving this branch from teonbrooks to makebrainwaves. Should be easier to bring through the home stretch that way.

@teonbrooks teonbrooks deleted the python_overhaul branch July 2, 2021 21:31
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