feat: add jupyter widgets and demo notebook for python support #16
Conversation
There was a problem hiding this comment.
@mkery Thank you very much for working on the Python support! Unfortunately, I could not reproduce the steps in py-notebook/README.md. I added GitHub comments to the specific commands that went wrong.
Also, I don't have any experience with creating extensions for Jupyter notebooks, but maybe that's a good thing for going through the documentation 🙈 .
py-notebook/README.md
Outdated
|
|
||
| 1. In a terminal, navigate to the included jupyter extension folder: | ||
|
|
||
| `$ cd neo-confusion-jupyter` |
There was a problem hiding this comment.
I had to run this command from the jupyter-widget folder.
|
|
||
| 2. Locally install the neo jupyter extension package to Python: | ||
|
|
||
| `$ pip install -e .` |
There was a problem hiding this comment.
When I call this (from the jupyter-widget folder), I get the following webpack error:
ERROR in main
Module not found: Error: Can't resolve './lib/embed.js' in '/Users/.../ml-hierarchical-confusion-matrix/py-notebook/jupyter-widget/js'
domoritz
changed the title
|
Yeah, that would be cool as it would also document the required steps. |
Co-authored-by: Yannick Assogba <yassogba@apple.com>
| @@ -0,0 +1,52 @@ | |||
| { | |||
| "name": "neo-confusion-jupyter", | |||
There was a problem hiding this comment.
The corresponding lockfile should ideally be at the top level as well. We can use yarn workspaces to make this work.
Without workspaces, we will need to keep the lock files in sync.
There was a problem hiding this comment.
Could you say more about why they should be in sync? They are effectively different packages, and there doesn't need to be local links between the two (see deps section below).
There was a problem hiding this comment.
Left a few inline comments for future discussion. But the bigger design related question I'm wondering is if there is a way to not need to release the neo-confusion-jupyter javascript package? Is it possible to just bundle the built code into the python extension itself. If so that would be one less package to maintain and a likely easier release process. For context I'm not particular familiar with Jupyter extensions.
A related question I had is in the current setup who is responsible for yarn build on the widgets javascript? Does that happen on the user's machine (meaning they require npm) or at build time before upload to pypi?
This may work better to chat about synchronously, but if there are pointers that make sense to leave here that works too.
| - [node](http://nodejs.org/) | ||
|
|
||
| ```bash | ||
| npm install --save neo-confusion-jupyter |
There was a problem hiding this comment.
Could you say a bit more about this? I'm not as familiar with jupyter widgets, so I'm wondering who is the consumer for this potential javascript package. Can the python package not include any necessary javascript?
|
|
||
| //console.log("RECIEVED DATA", data); | ||
|
|
||
| // TODO TEST ONLY |
There was a problem hiding this comment.
We should sync up on what's left for these TODOs (here and below)
| }, | ||
| externals: ['@jupyter-widgets/base'] | ||
| }, | ||
| {// Embeddable neo-confusion-jupyter bundle |
There was a problem hiding this comment.
I'm guessing this is related to the answer to my question above (about why neo-confusion-jupyter needs to be a package). How would people use this version? Is there a great advantage over the pip install approach?
| cmdclass=cmdclass, | ||
| author='Mary Beth Kery', | ||
| author_email='mkery@apple.com', | ||
| url='https://github.com/MI Vis/neo-confusion-jupyter', |
| - To release a new version of neo_confusion_jupyter on PyPI: | ||
|
|
||
| Update _version.py (set release version, remove 'dev') | ||
| git add the _version.py file and git commit |
There was a problem hiding this comment.
is this step already done? also we should update this to the full path.
| render() { | ||
| try { | ||
| // parse data | ||
| let dataStr = this.model.get("inputData"); |
There was a problem hiding this comment.
Consider const instead of let here and below.
@grtlr please try installing this notebook extension to verify my instructions work, thanks!
See the README.MD in the py-notebook folder for usage & install instructions.