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

IPython notebook with Sage Extensions #16996

Closed
vbraun opened this issue Sep 16, 2014 · 65 comments
Closed

IPython notebook with Sage Extensions #16996

vbraun opened this issue Sep 16, 2014 · 65 comments

Comments

@vbraun
Copy link
Member

vbraun commented Sep 16, 2014

Make the IPython html-based notebook work just like the sage command line.

In particular: make zeromq and pyzmq standard packages.

Screenshots: https://plus.google.com/photos/113188225509686176367/albums/6061181433704732017

Depends on #16746

CC: @jondo @videlec

Component: interfaces

Author: Volker Braun

Branch: c54edb5

Reviewer: Karl-Dieter Crisman, Sébastien Labbé, John Palmieri, Eric Gourgoulhon, Steve Singleton

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

@vbraun vbraun added this to the sage-6.4 milestone Sep 16, 2014
@vbraun
Copy link
Member Author

vbraun commented Sep 16, 2014

Branch: u/vbraun/sage_ipynb

@vbraun
Copy link
Member Author

vbraun commented Sep 16, 2014

comment:2

The first commit containts the anything-text-mode integration. Graphics still launches in a separate graphics viewer process.


New commits:

58ff710Add a sage -ipynb option
3c42208IPython notebook with Sage extensions

@vbraun
Copy link
Member Author

vbraun commented Sep 16, 2014

Commit: 3c42208

@vbraun
Copy link
Member Author

vbraun commented Sep 16, 2014

@vbraun
Copy link
Member Author

vbraun commented Sep 18, 2014

Dependencies: #16746

@vbraun

This comment has been minimized.

@kcrisman
Copy link
Member

comment:5

Aww, but sage -ipynb seems so cute. After all, it's the Ipython notebook, not the Notebook Ipython. Maybe both?

@vbraun
Copy link
Member Author

vbraun commented Sep 19, 2014

comment:6

Its cute. I also wanted to call the Parent object that owns the Singular ring "Frodo". Sometimes, you have to resist being cute ;-)

Since there is bound to be more than one notebook, I think its best to go with sage --notebook-foo or --notebook=foo. With just sage --notebook being some suitable default.

@kcrisman
Copy link
Member

comment:7

Such as sage --notebook-cloud, for example.

Well, it might still be good to make it more easily discoverable with an alias or something, just because of the English syntax order.

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member Author

vbraun commented Sep 20, 2014

comment:9

Also, you can lanuch the plain IPython notebook with sage -ipython notebook. The commandline switch for the Sage-enhanced IPython notebook should have more than one character difference ;-)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 20, 2014

Changed commit from 3c42208 to 95a4ea4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 20, 2014

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

48a2320Also call graphics.show() in doctest mode
fbf83ebShow output of the lonely generic_graph doctest
a9271f0Fix doctests that have dictionary keys without stable order
3922ed5Reduce doctest precision
b362aa6Display trailing newline in __repr__() output
7de159aFix more random test failures
b02f820Merge #16746 (Improvements to the Sage displayhook)
7db4081Make zeromq and pyzmq standard packages
e7e7f9cAdd 2d graphics to the ipython notebook
95a4ea4Extend IPython notebook templates

@vbraun
Copy link
Member Author

vbraun commented Sep 20, 2014

comment:11

2d graphics works, 3d graphics will be displayed as 2d render. IMHO this is good enough for a first stab at it.

I've extended the magic _graphics_() method hook that we already had in Sage so that you can specify a list of file formats that are acceptable as output. The _graphics_() hook then can pick the best choice amongst these possibilities. This works fine for the problem of finding a suitable graphics file format.

The same idea should be extended to text/html/latex output as well. The user specifies which formats he wants (or his frontend can display), and Sage then picks the best representation. I'll work on refactoring the _graphics_() hook into a more general solution (probably called _repr_rich()) on a followup ticket. Once we have this it would be easy to make html/tex output work in a configurable way.

@williamstein
Copy link
Contributor

comment:12

Replying to @vbraun:

2d graphics works, 3d graphics will be displayed as 2d render. IMHO this is good enough for a first stab at it.

I've extended the magic _graphics_() method hook that we already had in Sage so that you can specify a list of file formats that are acceptable as output. The _graphics_() hook then can pick the best choice amongst these possibilities. This works fine for the problem of finding a suitable graphics file format.

The same idea should be extended to text/html/latex output as well. The user specifies which formats he wants (or his frontend can display), and Sage then picks the best representation. I'll work on refactoring the _graphics_() hook into a more general solution (probably called _repr_rich()) on a followup ticket. Once we have this it would be easy to make html/tex output work in a configurable way.

I'm glad you're working on this, since I've been meaning to do something like this for Sage for a long time, since it is needed for SageMathCloud. Unfortunately, I'm concerned that this approach will cause me more trouble than help, at least for me. With SageMathCloud I have to customize all of the outputs you mention above, and I think the above approach (and what you've implemented in this patch), does not provide a way for me to do it. I would have to directly edit the source code of Sage, which I don't want to do (since it breaks people's custom installs). Right now, SMC monkey patches all of the show methods, except for 3d graphics, which can't be monkey patched, due to being in Cython. The idea that "show" can be implemented by saving to a file format, then calling some other function is insufficient, since in some cases one wants to query an object in various ways as part of show. What is really needed is a hook so that users can replace the show method altogether at runtime if they want.

So if you can do all of the above, but also provide a hook for replacing every show method by one of the user's choosing, especially supporting 3d graphics which can't be monkey patched, then I'm on board.

@vbraun
Copy link
Member Author

vbraun commented Sep 29, 2014

comment:13

Monkey patching stuff left and right isn't going to be maintainable in the long run anyways, so sooner or later you'll have to implement a proper solution. And that means a compute kernel starting with a custom displayhook or piggy-backing on the Sage-IPython ZMQ kernel. I can't see the SMC code so I can't really help with that, but I do intend to write the necessary framework to make implementing the displayhook easy. And powerful enough to include graphics, latex, and html output if desired.

@williamstein
Copy link
Contributor

comment:14

Replying to @vbraun:

Monkey patching stuff left and right isn't going to be maintainable in the long run anyways,
so sooner or later you'll have to implement a proper solution.

It's actually been very easy to maintain for almost two years now, since Sage's 2d and 3d graphics, etc., are all VERY stable code for the last 3 years. However, I don't like doing it. Amusingly, what you're doing now is the first situation that could make it harder.

And that means a compute kernel starting with a custom displayhook or piggy-backing on the Sage-IPython ZMQ kernel. I can't see the SMC code so I can't really help with that,

All relevant Python code is BSD or GPL'd licensed. The latest versions are always the Python files in ~/.sagemathcloud in any SageMathCloud project. If you want me to post a snapshot somewhere else I can.

SageMathCloud makes absolutely no use of ZMQ or IPython. However, it uses a custom displayhook.

but I do intend to write the necessary framework to make implementing the displayhook easy. And powerful enough to include graphics, latex, and html output if desired.

Cool -- that's exactly what I want -- I way to make the necessary framework for implementing the displayhook easy, instead of just replacing show methods.

Anyway, I'm thrilled you're working on this, and I hope you'll keep SMC in mind for your design, so that I can rewrite my code to use your framework instead of monkey patching.

@seblabbe
Copy link
Contributor

comment:16
  1. [needs work] The sage -ipython notebook works OK for me. But the command sage -notebook-ipy is broken. There is a tab that opens in the browser but it stays empty. In the command line, it says templates tree.html and error.html and maybe others are missing :
$ sage -notebook-ipy
2014-09-30 23:29:19.288 [SageNotebookApp] Using existing profile dir: u'/Users/slabbe/.sage/ipython-2.2.0/profile_default'
2014-09-30 23:29:19.302 [SageNotebookApp] Using MathJax from CDN: https://cdn.mathjax.org/mathjax/latest/MathJax.js
2014-09-30 23:29:19.345 [SageNotebookApp] Serving notebooks from local directory: /Users/slabbe/.sage/notebooks_ipy
2014-09-30 23:29:19.345 [SageNotebookApp] 0 active kernels 
2014-09-30 23:29:19.345 [SageNotebookApp] The IPython Notebook is running at: http://localhost:8888/
2014-09-30 23:29:19.345 [SageNotebookApp] Use Control-C to stop this server and shut down all kernels (twice to skip confirmation).
2014-09-30 23:29:20.051 [tornado.application] ERROR | Uncaught exception GET /tree (::1)
HTTPRequest(protocol='http', host='localhost:8888', method='GET', uri='/tree', version='HTTP/1.1', remote_ip='::1', headers={'Connection': 'keep-alive', 'Accept-Language': 'fr-ca,fr;q=0.8,fr-fr;q=0.6,en-us;q=0.4,en;q=0.2', 'Accept-Encoding': 'gzip, deflate', 'If-None-Match': '"5a5567e3b37b9ccb8e61737610c32e684f6090d4"', 'Host': 'localhost:8888', 'Accept': 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', 'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:16.0) Gecko/20100101 Firefox/16.0'})
Traceback (most recent call last):
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/tornado-3.1.1-py2.7.egg/tornado/web.py", line 1141, in _when_complete
    callback()
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/tornado-3.1.1-py2.7.egg/tornado/web.py", line 1162, in _execute_method
    self._when_complete(method(*self.path_args, **self.path_kwargs),
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/tornado-3.1.1-py2.7.egg/tornado/web.py", line 2297, in wrapper
    return method(self, *args, **kwargs)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/tree/handlers.py", line 77, in get
    breadcrumbs=breadcrumbs
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/base/handlers.py", line 221, in render_template
    template = self.get_template(name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/base/handlers.py", line 217, in get_template
    return self.settings['jinja2_env'].get_template(name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/environment.py", line 716, in get_template
    return self._load_template(name, self.make_globals(globals))
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/environment.py", line 690, in _load_template
    template = self.loader.load(self, name, globals)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/loaders.py", line 115, in load
    source, filename, uptodate = self.get_source(environment, name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/loaders.py", line 180, in get_source
    raise TemplateNotFound(template)
TemplateNotFound: tree.html
2014-09-30 23:29:20.060 [tornado.application] ERROR | Uncaught exception in write_error
Traceback (most recent call last):
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/tornado-3.1.1-py2.7.egg/tornado/web.py", line 794, in send_error
    self.write_error(status_code, **kwargs)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/tornado-3.1.1-py2.7.egg/tornado/web.py", line 827, in write_error
    self.finish(self.get_error_html(status_code, **kwargs))
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/base/handlers.py", line 277, in get_error_html
    html = self.render_template('error.html', **ns)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/base/handlers.py", line 221, in render_template
    template = self.get_template(name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/IPython/html/base/handlers.py", line 217, in get_template
    return self.settings['jinja2_env'].get_template(name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/environment.py", line 716, in get_template
    return self._load_template(name, self.make_globals(globals))
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/environment.py", line 690, in _load_template
    template = self.loader.load(self, name, globals)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/loaders.py", line 115, in load
    source, filename, uptodate = self.get_source(environment, name)
  File "/Users/slabbe/Applications/sage-git/local/lib/python2.7/site-packages/Jinja2-2.5.5-py2.7.egg/jinja2/loaders.py", line 180, in get_source
    raise TemplateNotFound(template)
TemplateNotFound: error.html
2014-09-30 23:29:20.063 [tornado.access] ERROR | {
  "Accept-Language": "fr-ca,fr;q=0.8,fr-fr;q=0.6,en-us;q=0.4,en;q=0.2", 
  "Accept-Encoding": "gzip, deflate", 
  "Connection": "keep-alive", 
  "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", 
  "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:16.0) Gecko/20100101 Firefox/16.0", 
  "Host": "localhost:8888", 
  "If-None-Match": "\"5a5567e3b37b9ccb8e61737610c32e684f6090d4\""
}
2014-09-30 23:29:20.064 [tornado.access] ERROR | 500 GET /tree (::1) 16.67ms referer=None

  1. [needs info] When using the sage -ipython notebook which works for me. The Download as > rst throws me the following error:
500 : Internal Server Error
The error was: nbconvert failed: template file "rst" could not be found
  1. [suggestion] I do like sage -ipynb. I do not like sage -notebook-ipy because the part ipy is not natural. I feel I will need to check the doc of sage -advanced each time to confirm the name of the command. If you want the prefix notebook, I would rather suggest :
sage -notebook-ipython

@vbraun
Copy link
Member Author

vbraun commented Sep 30, 2014

comment:17

You need to run "make" to install the templates

@seblabbe
Copy link
Contributor

seblabbe commented Oct 1, 2014

comment:18

You are right. Running "make" fixed the problem. I can now open it and continue the review.

I do not know what goes in this ticket and what doesn't. For instance, as of now I believe the description of the ticket as been fulfilled ("IPython html-based notebook work just like the sage command line", "make zeromq and pyzmq standard packages"). And it could get a positive review.

But it would be nice if output like images would appear in the notebook as it is suggested in the screeshot. Also, I have some problems viewing the documentation. More precisely,

  1. Graphics shows just below the cell which is great: plot(x^2,0,10) but using plot(x^2,0,10).show() opens in an outside window. Is this a bug, or intended?

  2. Animate opens in a outside window as well:

sage: t = var('t')
sage: a = animate((cos(c*pi*t) for c in sxrange(1,2,.2)))
sage: a.show()
  1. 3d Graphics objects plots shows also below the cell as a Tachyon image and running .show opens in a outside window.

  2. I saw you added some links in the Help Menu. Unfortunately, I get 404: Not Found on the six links where one of them is http://localhost:8888/static/a_tour_of_sage/index.html.

@vbraun
Copy link
Member Author

vbraun commented Oct 1, 2014

comment:19

At least for now, it is intentional that show() opens external programs. For example that is currently the only way to see jmol 3d.

In the long run I think show() should just be removed. Really, it only serves two purposes: easier forkbombing while True: sphere().show() and functions that show plots while not playing nice with the rest of the graphics infrastructure. The only difference between def f(): return plot() and def f(): plot().show() is that the latter can't be combined/superimposed with other plots, a user can't change axis options, you can't reasonably call it in your own code, ...

The other reason is that there is no "stdout for graphics", so to my knowledge the IPython notebook doesn't have a separate output channel for graphics either. Other than returning an image via the displayhook, that is.

Of course the old notebook is entangled with the show() methods, e.g. 3d plots show() is a whole mess of spaghetti code. Fun fact: you can't save a jmol script to a file, you can only generate it on-the-fly inside show(). So my plan would be first to implement _rich_repr_() and then transition other code over.

@vbraun
Copy link
Member Author

vbraun commented Oct 1, 2014

comment:20

PS: For the documentation to work you need to build it: "make doc"

@vbraun
Copy link
Member Author

vbraun commented Oct 1, 2014

comment:21

I guess there is IPython.display to display graphics outside of the display hook, will look into that.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2014

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

ec42cdfdeprecate the -inotebook option

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2014

Changed commit from 75bc455 to ec42cdf

@vbraun
Copy link
Member Author

vbraun commented Oct 7, 2014

comment:41

I've added a (deprecated) to the inotebook option, hopefully we can get rid of it in the future.

@seblabbe
Copy link
Contributor

seblabbe commented Oct 8, 2014

comment:42

Replying to @vbraun:

  • Added the default to the help string.

Can the default (sagenb vs ipython) be changed by the user? If yes, how?

@seblabbe
Copy link
Contributor

seblabbe commented Oct 8, 2014

comment:43

It seems there is a space missing in the usage line :sage-notebook instead of sage -notebook.

$ sage -notebook -h
usage: sage-notebook [-h] [--log LOG] [--notebook [NOTEBOOK]]
$ sage -docbuild -h
Usage: sage -docbuild [OPTIONS] DOCUMENT (FORMAT | COMMAND)

I also wonder why -n is not listed first on the line (compare with sage -docbuild -h):

  --notebook [NOTEBOOK], -n [NOTEBOOK], -notebook [NOTEBOOK]

To me it is an important feature that sage -ipython notebook directly lands in the present working directory. And I will be disappointed if sage -notebook=ipython does not. What is the opinion of other users about this?

I am sory to say this, but to me if we default to --notebook-dir=DOT_SAGE/notebooks_ipython we follow stupidly a bad design choice of the previous notebook.

The standard IPython notebook command line options work

In any case, should we document in sage -notebook -h that the long list of options of sage -ipython notebook -h can be used?

@seblabbe
Copy link
Contributor

seblabbe commented Oct 8, 2014

comment:44

If DOT_SAGE/notebooks_ipython dir exists, sage -t src/sage/repl/notebook_ipython.py returns All tests passed!. But if the dir does not exist, then one test is failing:

sage -t src/sage/repl/notebook_ipython.py  # 1 doctest failed   
**********************************************************************                     
File "src/sage/repl/notebook_ipython.py", line 63, in sage.repl.notebook_ipython.SageNotebookApp.load_config_file
Failed example:
    app.load_config_file()    # random output                                                           
Exception raised:
    Traceback (most recent call last):
    ...
    TraitError: No such notebook dir: u'/Users/slabbe/.sage/notebooks_ipython' 

@vbraun
Copy link
Member Author

vbraun commented Oct 8, 2014

comment:45

The default can't be changed without editing the source. You can always make an alias if its too much effort to type out.

The notebook launch script is sage-notebook, so this is the name that appears in the usage: help. We could override it but IMHO that would be even more confusing. Can't really be fixed without #21.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2014

Changed commit from ec42cdf to 11326b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2014

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

9d034ccMore command line help description
11326b6Make doctests work even if you never ran the ipython notebook before

@vbraun
Copy link
Member Author

vbraun commented Oct 8, 2014

comment:47
  • The order of --notebook [NOTEBOOK], -n [NOTEBOOK] is fixed in sage-notebook and I prefer it that way (most descriptive first).

  • I also think its unproductive to start a discussion about whether or not we should start in a particular directory. This is what the old notebook does, if you don't like it then open a ticket or post on sage-devel. I don't really care either way, but the behavior has to match.

  • I fixed the doctest error when you never ran the ipython notebook before.

@seblabbe
Copy link
Contributor

seblabbe commented Oct 8, 2014

comment:48
  • I also think its unproductive to start a discussion about whether or not we should start in a particular directory.

Ok I agree. This should be discussed on sage-devel.

I agree with John Palmieri about the doctest coverage in the graphics_file class. I think it should be 100% doctested and should not be too long to do so. Other than that, to me the ticket can get a positive review. (Note: I will not have much internet time in the next few days.)

@vbraun
Copy link
Member Author

vbraun commented Oct 8, 2014

comment:49

I said already that the graphics_file module needs to be rewritten to also allow for other forms of rich output. I'm not going to document it and then erase it in the next ticket.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2014

Changed commit from 11326b6 to c54edb5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2014

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

c54edb5Better notebook-specific help

@seblabbe
Copy link
Contributor

Reviewer: Karl-Dieter Crisman, Sébastien Labbé, John Palmieri, Eric Gourgoulhon, ssinglet

@seblabbe
Copy link
Contributor

comment:51

I give this a positive review.

Unfinished discussions (reported later) concerns:

  • the default directory for sage -notebook=ipython to be discussed on sage-devel
  • A new ticket should be created for the graphics_file module to be rewritten

@vbraun
Copy link
Member Author

vbraun commented Oct 13, 2014

Changed reviewer from Karl-Dieter Crisman, Sébastien Labbé, John Palmieri, Eric Gourgoulhon, ssinglet to Karl-Dieter Crisman, Sébastien Labbé, John Palmieri, Eric Gourgoulhon, Steve Singleton

@vbraun
Copy link
Member Author

vbraun commented Oct 14, 2014

Changed branch from u/vbraun/sage_ipynb to c54edb5

@seblabbe
Copy link
Contributor

Changed commit from c54edb5 to none

@seblabbe
Copy link
Contributor

comment:54

Unfinished discussions (reported later) concerns:

  • the default directory for sage -notebook=ipython to be discussed on sage-devel

This is now #17203.

Volker I am sure you can fix #17203 much quicklier than me since you put your hands in those files recently. Will you have time to work on #17203 in the next days?

Sébastien

@kcrisman
Copy link
Member

kcrisman commented Nov 3, 2014

comment:55

See https://groups.google.com/d/msg/sage-release/xgmJ3nAcUOY/d9-MZLTyowYJ for some regressions/incompatible changes in the syntax for command line launch of sagenb, presumably introduced in this ticket.

@seblabbe
Copy link
Contributor

seblabbe commented Nov 3, 2014

comment:56

Replying to @kcrisman:

some regressions/incompatible changes in the syntax for command line launch of sagenb, presumably introduced in this ticket.

Should that be considered as a blocker ?

See also #17281 for another (easy) follow up ticket.

@vbraun
Copy link
Member Author

vbraun commented Nov 3, 2014

comment:57

this ticket is closed. Followup is #17280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants