Opened 2 years ago

Closed 22 months ago

Last modified 19 months ago

#21267 closed enhancement (fixed)

Port SageNB widgets/interact to Jupyter

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.0
Component: notebook Keywords:
Cc: charpent Merged in:
Authors: Jeroen Demeyer Reviewers: Emmanuel Charpentier
Report Upstream: N/A Work issues:
Branch: 1046695 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by jason)

Support old SageNB widgets in Jupyter notebooks. This is done by writing wrapper functions with the old SageNB names returning ipywidgets widgets. That is, we essentially do

def input_box(...):
    return ipywidgets.Text(...)

The current implementation should be 100% compatible with ipywidgets, just extending it with Sage functionality.

We also try to support the old SageNB @interact functionality. Many examples of SageNB interacts can be found on https://wiki.sagemath.org/interact and we try to support as much as possible in Jupyter.

These are the changes in functionality that I know of:

  1. slider(0,10) returns a slider which runs over the integers from 0 to 10 (in SageNB, this ran over floats from 0.0 to 10.0).
  1. range_slider() with rational numbers is not implemented: https://github.com/ipython/ipywidgets/issues/760 (now implemented in https://github.com/jupyter-widgets/ipywidgets/pull/1356)
  1. The output is updated only once when a button is clicked multiple times: https://github.com/ipython/ipywidgets/issues/763 (the needed hook to implement this is done in https://github.com/jupyter-widgets/ipywidgets/pull/1259)
  1. Labels do not support HTML, only plain text and LaTeX: https://github.com/ipython/ipywidgets/issues/817
  1. The visual presentation of widgets can be different.

Tarballs:

Change History (64)

comment:1 follow-up: Changed 2 years ago by vbraun

Are there any API conflicts? And if yes, how are we dealing with that? Separate @jupyterinteract or what? A global switch?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 2 years ago by jdemeyer

Replying to vbraun:

Are there any API conflicts?

At least, the API is mostly compatible. I don't know any case where SageNB and Jupyter have significantly different behaviour. For the things that SageNB supports but Jupyter does not, either drop support for that feature or work with upstream to implement the feature in Jupyter. I don't strive to achieve 100% compatibility, I'm happy if it mostly works.

In some cases, the visual presentation might be different (Jupyter using a drop-down box while SageNB uses something like radio buttons). I don't care about that.

One potential issue is that SageNB supports all kinds of numbers (Integer, RealNumber, Rational, ...) for sliders while Jupyter only supports int and float.

comment:3 Changed 2 years ago by jdemeyer

  • Branch set to u/jdemeyer/port_sagenb_widgets_interact_to_jupyter

comment:4 Changed 2 years ago by git

  • Commit set to f9e723b65a8ed6f87fdee4471a22f71b68330755

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f9e723bPort SageNB interacts to Jupyter

comment:5 follow-up: Changed 2 years ago by kcrisman

WOW how did I not know about this ticket!

comment:6 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:7 in reply to: ↑ 5 Changed 2 years ago by jdemeyer

Replying to kcrisman:

WOW how did I not know about this ticket!

I'll let you know when I need testers.

comment:8 Changed 2 years ago by charpent

  • Cc charpent added

I posted a temporary workaround for people wanting interactive graphs in Jupyter notebooks using the Sage kernel.

comment:9 Changed 2 years ago by jdemeyer

For the record, I consider this branch mostly feature-complete. It still lacks documentation and tests and the required changes to the ipywidgets package.

I have not decided what to do with the upstream situation. There are 3 options:

  • add a whole lot of patches to build/pkgs/ipywidgets
  • package the upstream master branch
  • wait for ipywidgets-6 to be released

comment:10 Changed 2 years ago by git

  • Commit changed from f9e723b65a8ed6f87fdee4471a22f71b68330755 to 8704fc0f4000250e4b4c9474c60c6877c56c10b7

Branch pushed to git repo; I updated commit sha1. New commits:

8704fc0Fix selector with labels

comment:11 in reply to: ↑ 2 ; follow-up: Changed 2 years ago by novoselt

Replying to jdemeyer:

... I don't strive to achieve 100% compatibility, I'm happy if it mostly works.

I say that it is quite important to work towards such a goal for all interfaces: SageNB, Jupyter, SageMathCloud, SageMathCell. It is highly annoying to develop a nice interact and then discover that it does not work with another interface. Perhaps SageNB can be left behind now, but I hope that other 3 can converge to a single documented standard...

comment:12 in reply to: ↑ 11 Changed 2 years ago by jdemeyer

Replying to novoselt:

I say that it is quite important to work towards such a goal for all interfaces: SageNB, Jupyter, SageMathCloud, SageMathCell. It is highly annoying to develop a nice interact and then discover that it does not work with another interface. Perhaps SageNB can be left behind now, but I hope that other 3 can converge to a single documented standard...

I agree that we should work towards such a goal but some things are not so easy at the moment. In particular, this patch is re-using the existing widgets from ipywidgets, it doesn't add completely new widgets (it doesn't change any Javascript). For example, ipywidgets doesn't have a slider with rational numbers so we cannot have that. Luckily, this can be emulated by a slider taking an arbitrary list of values. But this trick does not work for a range_slider, so a range_slider with rational numbers is not supported here.

comment:13 Changed 2 years ago by jdemeyer

IMHO, sliders behave strangely in SageNB:

@interact
def f(x=(1, 10)):
    print(x, parent(x))

This gives a slider with floating-point values between 1 and 10. In ipywidgets, this would become a slider with integer values between 1 and 10. I think that the ipywidgets convention makes more sense here: the type of the variable should correspond to the type in the interact abbrevation (x=(1, 10) gives integers while x=(1.0, 10.0) gives floats).

On top of this, this gives Python floats while in a Sage context, an element of RR would be more appropriate.

comment:14 Changed 2 years ago by git

  • Commit changed from 8704fc0f4000250e4b4c9474c60c6877c56c10b7 to 6b0dd4c15dd7656a18242918a808c3345f2806ce

Branch pushed to git repo; I updated commit sha1. New commits:

6b0dd4cAllow unicode labels

comment:15 Changed 2 years ago by git

  • Commit changed from 6b0dd4c15dd7656a18242918a808c3345f2806ce to 66decb63c93693b6ebd494ce9789b321107a47a9

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

66decb6Allow unicode labels

comment:16 Changed 2 years ago by jdemeyer

  • Dependencies changed from #21256 to #21552

comment:17 Changed 2 years ago by git

  • Commit changed from 66decb63c93693b6ebd494ce9789b321107a47a9 to 41e476d675819a8a122827c0ebb39040ace82ddd

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cf0621dFix Python bug #20108: getcallargs() with func keyword
41e476dPort SageNB interacts to Jupyter

comment:18 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 2 years ago by git

  • Commit changed from 41e476d675819a8a122827c0ebb39040ace82ddd to d8e04b196be4099539574e4272bc8b32ddcf6979

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d8e04b1Port SageNB interacts to Jupyter

comment:20 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:21 Changed 2 years ago by jason

  • Description modified (diff)

comment:22 Changed 2 years ago by jdemeyer

  • Dependencies #21552 deleted
  • Description modified (diff)
  • Milestone changed from sage-7.4 to sage-7.6

comment:23 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:24 Changed 2 years ago by jdemeyer

  • Dependencies set to #22119

comment:25 Changed 2 years ago by git

  • Commit changed from d8e04b196be4099539574e4272bc8b32ddcf6979 to 19cde93272ad976d2c82f2df84fecee9eb611a39

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6d0845du(): assume UTF-8
3809fffUpgrade ipywidgets
19cde93Port SageNB interacts to Jupyter

comment:26 Changed 2 years ago by jdemeyer

  • Dependencies changed from #22119 to #22119, #22125

comment:27 Changed 2 years ago by git

  • Commit changed from 19cde93272ad976d2c82f2df84fecee9eb611a39 to 147b9808f35f6624052c8641a4f61a66ad657801

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7d3707aUpgrade Jupyter packages, add enum34
bd84e65u(): assume UTF-8
1f0caa7Upgrade ipywidgets
147b980Port SageNB interacts to Jupyter

comment:28 Changed 2 years ago by git

  • Commit changed from 147b9808f35f6624052c8641a4f61a66ad657801 to 4957bbc0c71436104685619e550208c8c80b2eaa

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4957bbcPort SageNB interacts to Jupyter

comment:29 Changed 2 years ago by git

  • Commit changed from 4957bbc0c71436104685619e550208c8c80b2eaa to f443f89c308d159ce863e37857813c47826911a6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9e32c28Upgrade Jupyter packages, add enum34
e844814u(): assume UTF-8
fe7018bUpgrade ipywidgets
f443f89Port SageNB interacts to Jupyter

comment:30 follow-up: Changed 23 months ago by jdemeyer

  • Description modified (diff)

comment:31 in reply to: ↑ 30 Changed 23 months ago by charpent

Replying to jdemeyer:

Is this upgrade ready for review ? Or do you need any form of help before formal review ?

comment:32 Changed 23 months ago by jdemeyer

Essentially yes, it's ready.

I didn't formally set the ticket to needs_review because I'd like to wait until the stable release of ipywidgets is released.

comment:33 Changed 23 months ago by jdemeyer

  • Description modified (diff)

comment:34 Changed 23 months ago by jdemeyer

  • Description modified (diff)

comment:35 Changed 23 months ago by git

  • Commit changed from f443f89c308d159ce863e37857813c47826911a6 to 7ce92bbcbfdabfc8f9750ebdce079ddfd99df420

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7d15c46Upgrade Jupyter packages, add enum34
343abbfu(): assume UTF-8
9598944Upgrade ipywidgets
7ce92bbPort SageNB interacts to Jupyter

comment:36 Changed 23 months ago by git

  • Commit changed from 7ce92bbcbfdabfc8f9750ebdce079ddfd99df420 to 64ee490ff787c44d357ec456471196f065f584cd

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2b1e9fdUpgrade ipywidgets
64ee490Port SageNB interacts to Jupyter

comment:37 Changed 23 months ago by jdemeyer

  • Dependencies #22119, #22125 deleted

comment:38 Changed 23 months ago by jdemeyer

  • Description modified (diff)

comment:39 Changed 23 months ago by git

  • Commit changed from 64ee490ff787c44d357ec456471196f065f584cd to 6e6013aea58a92f2b242995b899a083b3d55a183

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8ec0760Upgrade ipywidgets
6e6013aPort SageNB interacts to Jupyter

comment:40 Changed 23 months ago by jdemeyer

  • Description modified (diff)

comment:41 Changed 22 months ago by jdemeyer

  • Description modified (diff)

comment:42 Changed 22 months ago by git

  • Commit changed from 6e6013aea58a92f2b242995b899a083b3d55a183 to 053ad423c67f9d23ff527cd3a1307e89d277dadd

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6890482Upgrade ipywidgets
053ad42Port SageNB interacts to Jupyter

comment:43 Changed 22 months ago by jdemeyer

  • Milestone changed from sage-7.6 to sage-8.0

comment:44 Changed 22 months ago by git

  • Commit changed from 053ad423c67f9d23ff527cd3a1307e89d277dadd to 30955d2ec72338cc7b13c17922389bb58c8f278c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2dbd6a5Upgrade ipywidgets
30955d2Port SageNB interacts to Jupyter

comment:45 Changed 22 months ago by jdemeyer

  • Status changed from new to needs_review

comment:46 Changed 22 months ago by charpent

  • Reviewers set to Emmanuel Charpentier
  • Status changed from needs_review to positive_review

starting with a freshly fetched 7.6.beta4, make distclean && make ptestlong passes with one well-known-failure :

----------------------------------------------------------------------
sage -t --long src/sage/homology/simplicial_complex.py  # 1 doctest failed
----------------------------------------------------------------------

which passes when ran standalone.

A couple warnings are repeated in widgetsnbextension-2.0.0rc1.log and ipywidgets-6.0.0.rc2.log :

/usr/local/sage-exp/local/lib/python2.7/site-packages/setuptools/dist.py:285: UserWarning: Normalizing '2.0.0.rc1' to '2.0.0rc1'
warning: manifest_maker: standard file '-c' not found
/usr/local/sage-exp/local/lib/python2.7/site-packages/setuptools/dist.py:285: UserWarning: Normalizing '6.0.0.rc2' to '6.0.0rc2'

which look like innocuous booboos.

==> positive_review

For the record : congratulations for this intricate, lengthy and much expected patch. Thank you very much !

I'd recommend expedited inclusion of this update, which should go a long way making the Jupyter notebook as functional as the old one.

I plan to merge this one in my current working branch (which includes #20523 an its dependencies) and give it a couple "real-life" functional tests. But unless the results turns out to be non- or barely-functional (which would surprise me), further enhancements should go towards a new ticket.

comment:47 follow-ups: Changed 22 months ago by charpent

  • Status changed from positive_review to needs_work

Damn !

from the PREP tutorial "Sage Interact Quickstart" :

sage: @interact
sage: def _(f=input_box(x^2,width=20),
....: color=color_selector(widget='colorpicker', label=""),
....: axes=True,
....: fill=True,
....: zoom=range_slider(-3,3,default=(-3,3))):
....:     show(plot(f,(x,zoom[0], zoom[1]), color=color, axes=axes,fill=fill, figsize=3))

(I added the figsize=3)

gives a static fgure, with no controls. I see that in the terminal windows from where I launched the notebook :

[IPKernelApp] WARNING | The installed widget Javascript is the wrong version. It must satisfy the semver range ~2.1.0.

One hidden dependency ? Something to document (in plain English...) ?

Or something more serious ?

BTW : this works well in the old Sage notebook (same binary, ran from another terminal...).

comment:48 in reply to: ↑ 47 ; follow-up: Changed 22 months ago by jdemeyer

Replying to charpent:

Damn !

from the PREP tutorial "Sage Interact Quickstart" :

sage: @interact
sage: def _(f=input_box(x^2,width=20),
....: color=color_selector(widget='colorpicker', label=""),
....: axes=True,
....: fill=True,
....: zoom=range_slider(-3,3,default=(-3,3))):
....:     show(plot(f,(x,zoom[0], zoom[1]), color=color, axes=axes,fill=fill, figsize=3))

(I added the figsize=3)

gives a static fgure, with no controls. I see that in the terminal windows from where I launched the notebook :

[IPKernelApp] WARNING | The installed widget Javascript is the wrong version. It must satisfy the semver range ~2.1.0.

One hidden dependency ? Something to document (in plain English...) ?

Your example works for me. My guess is that it's something like a browser cache which might interfere. Can you try clearing the cache and try again?

Last edited 22 months ago by jdemeyer (previous) (diff)

comment:49 in reply to: ↑ 48 ; follow-ups: Changed 22 months ago by charpent

Replying to jdemeyer:

Replying to charpent:

[ Snip ...]

Your example works for me. My guess is that it's something like a browser cache which might

interfere. Can you try clearing the cache and try again?

Done. Same result.

I might have another problem : when I type "SAGE_BROWSER=chromium sage -n jupyter", Sage still opens a Firefox sheet.

Logging out of this sheet, manually opening Chromium, and pasting the address given in the controlling terminal window allows to open the same sheet in Chrome, with the same result.

Suggestions ?

comment:50 in reply to: ↑ 49 Changed 22 months ago by charpent

One more data point : same results (incl. Firefox starting with SAGE_BROWSER="chromium") after rebooting ths whole hawg...

I'm at loss.

comment:51 in reply to: ↑ 49 ; follow-up: Changed 22 months ago by jdemeyer

Replying to charpent:

Suggestions ?

This is most likely not Sage-related, so please report this issue upstream to ipywidgets: https://github.com/ipython/ipywidgets/issues

comment:52 in reply to: ↑ 51 Changed 22 months ago by charpent

Replying to jdemeyer:

Replying to charpent:

Suggestions ?

This is most likely not Sage-related, so please report this issue upstream to ipywidgets: https://github.com/ipython/ipywidgets/issues

This is now ipywidget's issue 1611.

comment:53 Changed 22 months ago by jdemeyer

  • Description modified (diff)

comment:54 Changed 22 months ago by git

  • Commit changed from 30955d2ec72338cc7b13c17922389bb58c8f278c to 10466954904447ad0df31bc559acb36121e4b40c

Branch pushed to git repo; I updated commit sha1. New commits:

1046695Fix exceptions in interacts

comment:55 in reply to: ↑ 47 Changed 22 months ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to charpent:

[IPKernelApp] WARNING | The installed widget Javascript is the wrong version. It must satisfy the semver range ~2.1.0.

Do you happen to have another installation of ipywidgets/widgetsnbextension which might interfere?

I'm setting this back to needs_review because I cannot reproduce your problem. I hope that other people will try it, so we can see if it works for them.

comment:56 Changed 22 months ago by charpent

  • Status changed from needs_review to positive_review

I tried your patch on two different installations (one physical machine, one pristine VM) where it succeeded. Coming back to my initial test case, I took the speps to remove entierely ipywidgets an widgetsnbextension from the systemwide installs. Theren your patch worked.

I conclude that, somehow, the systemwide installation(s) of Python extensions take precedence over a sage-specific installation. hence a couple questions :

  • Is that expected or is that a bug ?
  • If a bug, does it deserve a ticket ?
  • If expected (not a bug), shouldn't this behavior be documented ?

In any case, This lifts my objections ==> positive_review.

And kudos !

comment:57 Changed 22 months ago by jdemeyer

Upstream is aware of issues with conflicting installations. In fact that's why I made the suggestion in 55 to check other installations.

comment:58 Changed 22 months ago by vbraun

  • Branch changed from u/jdemeyer/port_sagenb_widgets_interact_to_jupyter to 10466954904447ad0df31bc559acb36121e4b40c
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:59 follow-up: Changed 22 months ago by fbissey

  • Commit 10466954904447ad0df31bc559acb36121e4b40c deleted

@ Jeroen: quick question, I notice you have added two patches for ipywidgets. Are those upstreamed? If so can you give some references?

comment:60 in reply to: ↑ 59 ; follow-up: Changed 22 months ago by jdemeyer

Replying to fbissey:

@ Jeroen: quick question, I notice you have added two patches for ipywidgets.

Actually, there are 3 patches. Two of them (get_interact_value_output.patch and output_exception.patch) should be in https://pypi.python.org/pypi/ipywidgets/6.0.0.rc3

The other one (widget_repr.patch) is being discussed at https://github.com/ipython/ipywidgets/pull/1031 but upstream isn't very happy about it. However, this one is really only relevant for doctesting, not for runtime functionality.

comment:61 in reply to: ↑ 60 Changed 22 months ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

@ Jeroen: quick question, I notice you have added two patches for ipywidgets.

Actually, there are 3 patches. Two of them (get_interact_value_output.patch and output_exception.patch) should be in https://pypi.python.org/pypi/ipywidgets/6.0.0.rc3

OK, good!

The other one (widget_repr.patch) is being discussed at https://github.com/ipython/ipywidgets/pull/1031 but upstream isn't very happy about it. However, this one is really only relevant for doctesting, not for runtime functionality.

OK, I missed that one completely, I suppose that is why I have

File "/usr/lib64/python2.7/site-packages/sage/repl/ipython_kernel/widgets.py", line 349, in sage.repl.ipython_kernel.widgets.Grid.__init__
Failed example:
    w
Expected:
    Grid(value=[[0, 1], [4, 5]], description=u'2x2 matrix', children=(Label(value=u'2x2 matrix'), VBox(children=(EvalText(value=u'0'), EvalText(value=u'4'))), VBox(children=(EvalText(value=u'1'), EvalText(value=u'5')))))
Got:
    <sage.repl.ipython_kernel.widgets.Grid object at 0x7f8d65145f90>

And indeed it doesn't seem to have an impact on functionality.

comment:62 Changed 21 months ago by jdemeyer

Follow-up: #22550, please review

comment:63 Changed 19 months ago by jason

  • Description modified (diff)

I updated the description to reflect recent changes in upstream ipywidgets.

comment:64 Changed 19 months ago by jdemeyer

Thanks for the pointers, I will certainly follow up on this (but maybe not right now).

Note: See TracTickets for help on using tickets.