jupytercalpoly / reactivepy

A reactive Python kernel
BSD 3-Clause "New" or "Revised" License
86 stars 11 forks source link

Add binder support #30

Closed gnestor closed 5 years ago

gnestor commented 5 years ago

Closes https://github.com/jupytercalpoly/reactivepy/issues/29

To try binder before merging this: https://mybinder.org/v2/gh/gnestor/reactivepy/binder?urlpath=lab/tree/examples/BasicAsyncGenerators.ipynb

ellisonbg commented 5 years ago

Hmmm, the link you provided to try out the binder worked, but when the notebooks came up the reactive kernel notebooks did not seem to work.

On Tue, Dec 4, 2018 at 12:20 PM Grant Nestor notifications@github.com wrote:

Closes #29 https://github.com/jupytercalpoly/reactivepy/issues/29

To try binder before merging this: https://mybinder.org/v2/gh/gnestor/reactivepy/binder?urlpath=lab/tree/examples/BasicAsyncGenerators.ipynb

You can view, comment on, or merge this pull request online at:

https://github.com/jupytercalpoly/reactivepy/pull/30 Commit Summary

  • Add binder support

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jupytercalpoly/reactivepy/pull/30, or mute the thread https://github.com/notifications/unsubscribe-auth/AABr0NPSCjXx1gPX-zYparsGd139rtgdks5u1tj3gaJpZM4ZBbrc .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

gnestor commented 5 years ago

I'm seeing the same. The regular Python kernel isn't working either. Unfortunately, can't inspect the terminal output in binder 🤔I'm seeing that the websocket connection is disconnecting in the browser console.

Simply running pip install -e . locally installs it properly for me.

Any ideas? Could the requirements.txt be causing problems?

ellisonbg commented 5 years ago

I am guessing it is something related to the installation of packages, including ipython and the reactive kernel.

gnestor commented 5 years ago

Ok, I set a tornado minimum version dependency and now it's working 👍

ellisonbg commented 5 years ago

Testing...

ellisonbg commented 5 years ago

Worked, but looks like there are a few other libraries that need to be installed for the example notebooks to full run (aiohttp, and maybe others)...

gnestor commented 5 years ago

I'm seeing this:

$ show_graph()
Traceback (most recent call last):
  File "/home/jovyan/reactivepy/kernel.py", line 392, in _update_kernel_state
    raise e
  File "/home/jovyan/reactivepy/kernel.py", line 386, in _update_kernel_state
    code_obj, cell_id)
  File "/home/jovyan/reactivepy/kernel.py", line 312, in _create_new_exec_unit
    raise DefinitionNotFoundException(str(sym))
DefinitionNotFoundException: [show_graph]

Any idea what's causing this?

ellisonbg commented 5 years ago

Where is the show_graph() function being imported from?

gnestor commented 5 years ago

That's a great question! I assumed it was some global provided by the kernel but I don't see it anywhere... 🤔

ellisonbg commented 5 years ago

This is the only other reference I see:

https://github.com/jupytercalpoly/reactivepy/blob/490ccc33a4c05adf2a4e1eb37c17d65b9703732a/reactivepy/code_object.py#L70

Let's maybe remove it for now from the example notebook.

gnestor commented 5 years ago

@richagadgil Do you have any idea where this show_graph function is defined or where it comes from? If not, is it ok to remove it?

declanvk commented 5 years ago

@gnestor I think the show_graph function was replace with var_dependency_graph or cell_dependency_graph in https://github.com/jupytercalpoly/reactivepy/blob/831f519a012194d446e1ee5f0494c28da8a58659/reactivepy/kernel.py#L221-L223

In either case it can be deleted from the example notebook

gnestor commented 5 years ago

Thanks @declanvk!

gnestor commented 5 years ago

Ok, it's working now 👍

gnestor commented 5 years ago

This should be ready to merge 👍