sagemath/sage

convert mandelbrot/julia interact to jupyter notebook

fchapoton opened this issue · 56 comments

as another step to python3

Depends on #25373

CC: @bbarros50 @bhutz @sagetrac-atowsley @kcrisman @embray @jdemeyer @jm58660

Component: notebook

Author: Frédéric Chapoton, Jeroen Demeyer

Branch/Commit: 4de168d

Reviewer: Frédéric Chapoton, Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/24994

comment:1

work in progress, not yet working as expected..


New commits:

a9e184dfirst tentative of conversion of mandelbrot interact to jupyter interact

Commit: a9e184d

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

52ce4a5second pass at interactive mandelbrot/julia in ipython notebook

Changed commit from a9e184d to 52ce4a5

Changed commit from 52ce4a5 to c65a01c

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

c65a01cfixing colors and other details

Changed commit from c65a01c to 7cbf77b

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

7cbf77btrying to fix doctests..
comment:5

ok, working now. Except 3 failing doctests that I just fail to write using ellipsis !

Author: Frédéric Chapoton

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

4467110pep8 and pyflakes cleanup

Changed commit from 7cbf77b to 4467110

Changed commit from 4467110 to 588ac04

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

588ac04fixing doctests (ugly)
comment:9

This is bad:

M = mandelbrot_interact
...adjust various attributes of M...
return M

The problem is that you are changing and returning a unique global object. Instead of having

@interact.widget
def mandelbrot_interact(...

it would be better to just define mandelbrot_interact as an ordinary function and call interact.widget() inside the mandelbrot_plot function. That way, you end up with non-unique widgets.

Can you explain the <CSI-2K> appearing in the doctests? I feel uncomfortable accepting that without knowing where it comes from.

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

8bdbb0dmade the suggested changes

Changed commit from 588ac04 to 8bdbb0d

comment:11

Better like that ?

comment:12

ping ?

comment:13

ping ?

comment:14

review, please ?

comment:15

ping again

comment:16

Could please someone have a look at the result here ? This seems to work well, and is a good step in getting rid of the dependencies to sagenb.

comment:17

It looks good to me. I had just moved the imports from sagenb inline, but this is obviously a more complete approach. I haven't tested it, but if you say it works it's good enough for me.

comment:18

It seems not to be working anymore.. :(

comment:19

Jeroen, any idea of what should be done, please ?

comment:20

Here is the message I get (once disactivating the image):

Failed to display Jupyter Widget of type sage_interactive.

If you're reading this message in the Jupyter Notebook or JupyterLab Notebook, it may mean that the widgets JavaScript is still loading. If this message persists, it likely means that the widgets JavaScript library is either not installed or not enabled. See the Jupyter Widgets Documentation for setup instructions.

If you're reading this message in another frontend (for example, a static rendering on GitHub or NBViewer), it may mean that your frontend doesn't currently support widgets.

This is not specific to the widget built here. Simple basic widgets have the same issue. Could someone confirm this issue ?

import ipywidgets as widgets
widgets.Checkbox()
comment:22

I can see no problem with mandelbrot_plot(interact=True) on sage --notebook=ipython; another thing is sage --notebook=sagenb which just says "Interactive function <function mandelbrot_interact . . ."

But the main idea of fractal does not work when there is no widget for infinite zooming.

comment:23

If it works for you (in which version of sage ?), then one can probably set this to positive review. My install should be somehow broken.

Yes, this is a pity that there is no good clickable-picture widget. But the main aim here is to move towards python3 by geeting rid of all imports of sagenb.

comment:24

Replying to @fchapoton:

If it works for you (in which version of sage ?), then one can probably set this to positive review. My install should be somehow broken.

I had 8.2rc4, and with git trac checkout... of this ticket I got 8.2beta8. So should I try to merge this to newest beta/rc version?

Yes, this is a pity that there is no good clickable-picture widget. But the main aim here is to move towards python3 by geeting rid of all imports of sagenb.

OK.

comment:25

Replying to @jm58660:

So should I try to merge this to newest beta/rc version?

I did this. No changes, it wotks.

comment:26

Thanks a lot. Feel free to set to positive, then.

comment:27

Hmm... I don't know about those widgets. The code seems clear. Jeroen, have you checked this?

comment:28

Replying to @jm58660:

Jeroen, have you checked this?

No, but I guess I should...

Dependencies: #25074

Changed author from Frédéric Chapoton to Frédéric Chapoton, Jeroen Demeyer

comment:29

I'm not so happy that the input to mandelbrot_plot() is now completely ignored when interact=True is given. I'll try to work on this.

I will rebase on #25074. That's not strictly needed, but it's good to test against the newest version of ipywidgets.

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

ad085dcupgrade to ipywidgets 7.2.0 and widgetsnbextension 3.20
4b925befirst tentative of conversion of mandelbrot interact to jupyter interact

Changed commit from 8bdbb0d to 4b925be

comment:31

Is this still "needing work", Jeroen ?

This is the place where docbuild currently fails on the python3 patchbot. See

https://patchbot.sagemath.org/log/0/Ubuntu/18.04/x86_64/4.15.0-20-generic/petitbonum/2018-05-15%2012:37:45?plugin=docbuild

Changed dependencies from #25074 to none

comment:33

This is working fine, the only issue is comment:29

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

8143016first tentative of conversion of mandelbrot interact to jupyter interact

Changed commit from 4b925be to 8143016

comment:35

OK, I'll work on this today.

Dependencies: #25373

Changed commit from 8143016 to e4523c4

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

2e75d4cAdd IPython support for Cython functions
f25a0e8first tentative of conversion of mandelbrot interact to jupyter interact
e4523c4Improve Mandelbrot/Julia interact

Changed commit from e4523c4 to 56b571f

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

56b571fImprove Mandelbrot/Julia interact

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

4de168dImprove Mandelbrot/Julia interact

Changed commit from 56b571f to 4de168d

Reviewer: Frédéric Chapoton, Jeroen Demeyer

comment:41

I think I can reasonably set this to positive now.

Changed branch from public/24994 to 4de168d