Htmlnotebook #705

merged 232 commits into from Aug 23, 2011


None yet

5 participants

IPython member

This is the initial version of the notebook and the related notebook format. The main parts of the code that need review are:

  • IPython.nbformat.
  • IPython.frontend.html.notebook
  • IPython.core.display
ellisonbg and others added some commits Jul 14, 2011
@ellisonbg ellisonbg Fixing code to assume msg_type and msg_id are top-level.
* I have gone through and looked for instances of ['msg_type'] and
  ['msg_id'] and tried to make sure that I added ['header'] so
  pull the values out of the header.
* But there are many cases where I can't tell if the dict is the
  full message or the header already. This is especially true
  of the msg_id in the parallel db parts of the code.
* Tests pass, but this is scary.
@ellisonbg ellisonbg Renaming unpack_message to unserialize and updating docstrings. efa1f33
@ellisonbg ellisonbg Fixing docstrings and a few more places for msg_id/msg_type. d86d45f
@ellisonbg ellisonbg Adding html notebook subpackage. 0494d13
@ellisonbg ellisonbg Initial draft of HTML5/JS/CSS3 notebook. adcfbf8
@ellisonbg ellisonbg Adding shift-enter support. 9e95afe
@ellisonbg ellisonbg Fixing selection and focus logic. 04da04e
@ellisonbg ellisonbg Added placeholder text for TextCell. 927f44b
@ellisonbg ellisonbg Adding preventDefault to shift-up/down events. d7f2add
@ellisonbg ellisonbg Work on the server side of the html notebook. 398f176
@ellisonbg ellisonbg Basic server for htmlnotebook working. b7248bf
@ellisonbg ellisonbg Initial reply handling implemented along with css fixes. 0bfc754
@ellisonbg ellisonbg Make the main notebook div auto scroll. 5a8489e
@ellisonbg ellisonbg Adding tests for zmq.session. a09b42a
@ellisonbg ellisonbg More tests for Session.send/recv. b95a185
@ellisonbg ellisonbg Fixing messaging docs to reflect msg_type in header. fa7cc46
@ellisonbg ellisonbg Adding temp refs to msg_type to prevent nested dict gets. e5d9e2c
@ellisonbg ellisonbg Fixing bug in related to msg_type refactoring. e4a965d
@ellisonbg ellisonbg Fixing another bug in msg_type refactoring. f0d2271
@ellisonbg ellisonbg Server side of file based notebook store implemented. 5dc428b
@ellisonbg ellisonbg Initial template for notebook browser. a2dd039
@ellisonbg ellisonbg Added message to help users open notebook. 74b8ea8
@ellisonbg ellisonbg Updating jquery UI and themes. 1497e96
@ellisonbg ellisonbg Fixing main toolbar area and cleaning up jquery themes. 339135b
@ellisonbg ellisonbg Different clients now share a single zmq session.
Previously, each client (browser window) would open its own set
of ZMQ sockets to a kernel. Now one master set of connections
to the kernel is created and all clients share those connections.
In some ways, this simplifies the URL design.

I have also made kernel_ids server-side created.
@ellisonbg ellisonbg Minor changes to notebook css. df52f21
@ellisonbg ellisonbg Interrupt and restart work for kernels. 7731a50
@ellisonbg ellisonbg Cleaned up kernel action interface.
* Using POST rather than GET.
* Using simple URLs rather than a query string.
* Using a regexp to match the action.
@ellisonbg ellisonbg Status monitoring added to notebook.
* Busy == red
* Idle == gray
* Restarting == black
@ellisonbg ellisonbg Fixes to notebook scrolling and layout.
We are now using the flexible box model, so the layout won't work
on IE9, but it already doesn't work on IE9 because of WS support.
@ellisonbg ellisonbg Tweaking and cleanup of notebook.css.
* Adjusted font sizes for improved readability.
* Got rid of unused css attributes and styles.
* Changed font of Title to Verdana.
@ellisonbg ellisonbg Further font adjustments to the notebook.
* Using Fernando's recommended monospaced font list.
* Increased tool/tab menus to 10 pt.
@ellisonbg ellisonbg Work on the notebook's code cell.
* Cleaned up the js code for creating the code cell.
* Added a div around the input text area to allow the text area
  to have a width of 100%.
* Added CodeCell.toJSON.
@ellisonbg ellisonbg Basic notebook saving and loading.
* The logic in the server and javascript frontend is there for
  a basic JSON notebook format with a .ipynb extension.
* To save a new notebook: "%notebook save filename.ipynb"
* To save a notebook that is already saved: "%notebook save"
* To load a notebook from the cwd: "notebook load filename.ipynb"
@ellisonbg ellisonbg Initial latex printing for sympy and fixes to autogrow. 68db752
@ellisonbg ellisonbg Refactoring of the output and display system.
* LaTeX rendering now works.
* Font's are reworked in the css style sheet.
* Testing on FF and Chrome.
@ellisonbg ellisonbg Fixes to the latex rendering by adjusting the MathJax config. 6c644c6
@ellisonbg ellisonbg Basic exception display in the notebook is working. bffb119
@ellisonbg ellisonbg Added basic styling to text cells. 9447eb0
@ellisonbg ellisonbg Cells call grow by hand when they reload from JSON. a563b92
@minrk minrk include html frontend in packages/package_data
chmod +x ipython-notebook script
@ellisonbg ellisonbg Shift-Enter only selects the next cell if it is a CodeCell. e341cf4
@ellisonbg ellisonbg Fine tuning of notebook styles.
* Padding added to L/R of notebook div (40px).
* Margin added to T/B of cells (15px).
* Margin added to T of output div (15px).
* More elements using border-box layout mode.
@ellisonbg ellisonbg Adding logic to look for CDN version of MathJax and fallback to local. 5e55e20
@ellisonbg ellisonbg Added note about tornado version to main script. ff6dc82
@ellisonbg ellisonbg Minor fix to sympy latex printing.
* \dag is converted to \dagger.
@ellisonbg ellisonbg Adding new notebook examples.
* Many that use sympy's quantum computing (github master required)
* One from Fernando Perez that does text analysis.
@ellisonbg ellisonbg Minor changes to text_analysis notebook example. 1ff5a82
@ellisonbg ellisonbg Updating notebook examples, and notebook js/html. 264b061
@ellisonbg ellisonbg General CSS cleanup.
* Created layout.css for common layout related mixins.
* monospace is the default font for now.
@ellisonbg ellisonbg CodeMirror code editing added.
* codemirror2 repo has been added to IPython.
* Custom coloring used to match the qtconsole.
@ellisonbg ellisonbg Better handling of up/down arrows for CodeCells. e270403
@ellisonbg ellisonbg Much improved nagivation for the notebook cells.
* Up/Down arrow now used to navigate cells.
* For text cells, shift-enter renders, enter edits.
@ellisonbg ellisonbg Updating notebook examples. c9ad4e8
@ellisonbg ellisonbg Updating sympy notebook examples. b2aa21f
@ellisonbg ellisonbg Updating examples notebooks. 0f16a10
@ellisonbg ellisonbg Adding new density of states notebook example. b55bc76
@ellisonbg ellisonbg Creating files to new notebook app. 98f7851
@ellisonbg ellisonbg Fixing import statments in handlers and notebookapp. a94f1fa
@ellisonbg ellisonbg Initial reorg of files complete.
I have moved things around to get ready for making the notebook
a full blown app, but have not actually made it an app.  That is
@ellisonbg ellisonbg Renaming NotebookApplication to NotebookWebApplication. 8deb465
@ellisonbg ellisonbg Refactored htmlnotebook session and kernel manager.
* The KernelManager now manages multiple kernels with a uniform
* The SessionManager now manages the full set of channels+streams
  and the IPython.zmq.session.Session object for a single kernel.
@ellisonbg ellisonbg Refactoring the notebook app to support the new config system. 9ce4875
@ellisonbg ellisonbg Notebook app debugging.
* Logging is now working with a default of INFO.
* Other misc bug fixes.
@ellisonbg ellisonbg Work to adapt routers to new Session message protocol. f0ab259
@ellisonbg ellisonbg More work on updating the notebook zmq forwarding. fd1d84c
@ellisonbg ellisonbg Updating the notebook to work with the latex master.
* PNG figures are now used for matplotlib.
* NEW msg format is used where msg_type is in the header.
* Session is used for sending/recving.
@ellisonbg ellisonbg Splitting notebook.js into muliple files for development ease. 82dfeed
@ellisonbg ellisonbg Updating main notebook template to use split scripts. 90a9bbc
@ellisonbg ellisonbg Change unpack_message to unserialize in 27afdf3
@ellisonbg ellisonbg Implemented module and namespace pattern in js notebook. c5b0c06
@ellisonbg ellisonbg Using $.proxy to clean up callbacks. 73126bf
@ellisonbg ellisonbg Actually kill old kernels upon restart. 7fc2c96
@ellisonbg ellisonbg Starting to refactor the notebook layout ad2890a
@ellisonbg ellisonbg Initial payload handling.
* Syntax highlighting fixed for things like "math?".
* Basic pager is working.
@ellisonbg ellisonbg Refactoring pager into its own class. aa37ef2
@ellisonbg ellisonbg Updating jQuery to 1.6.2 and jQuery UI to 1.8.14.
I have also added a focus.html files for testing focus related
@ellisonbg ellisonbg Adding note about vbox related scroll bugs. ed4b3fd
@ellisonbg ellisonbg More work updating the notebook to use dynamics resizing. a0257f5
@ellisonbg ellisonbg Pager is working again. 9262f15
@ellisonbg ellisonbg More accuract height calculations for the pager collapse/expand. e37fefa
@ellisonbg ellisonbg Left panel is now working. 450ed2f
@ellisonbg ellisonbg Improving the scrolling model. 1579023
@ellisonbg ellisonbg Initial draft of panel section and the cell section working. c6ab30b
@ellisonbg ellisonbg Minor fixes to fonts and spacing. a7ef503
@ellisonbg ellisonbg Controls in cell section have a solid layout. 71e8168
@ellisonbg ellisonbg Help section implemented and working. 0376908
@ellisonbg ellisonbg Minors fixes and initial work on save widget.
* Fixed width of buttons in left panel.
* Started to create a save widget for the header.
@ellisonbg ellisonbg Work on save widget, kernel status widget and notebook section. 67aa594
@ellisonbg ellisonbg Fixing execution related things.
* Extra enter on FF is fixed by hooking into CodeMirror's
  onKeyEvent hook. We now have CodeMirror ignore shift-enter
  completely as we handle it ourselves.
* The cell execution logic in notebook.js has been refactored and
  the Run All/Selected buttons have been hooked up.
@ellisonbg ellisonbg Updating font-sizing to use the YUI protocol. 0c51946
@ellisonbg ellisonbg Prevent shift-enter from propagating and performing default. d8294f0
@ellisonbg ellisonbg Colors now working in tracebacks and the pager.
For now I am just converting ANSI color escape sequences to HTML
<span> tags that have css classes for coloring.
@ellisonbg ellisonbg Added complete method of JS kernel object. 5829983
@ellisonbg ellisonbg Autocompletion working with CTRL-SPACE. dad53c3
@ellisonbg ellisonbg CTRL-ENTER now runs a cell in "terminal mode"
In this mode, a new cell is not created after the current cell
is run. Once the cell is run, the current input is cleared, so
it acts just like the terminal.
@ellisonbg ellisonbg Removing default input prompt number.
In a notebook setting being able to delete and add cells makes it
virtually impossible to correctly guess what the next input
prompt number should be. We now follow the convention that our
prompts look like "In [ ]:" before execution.
@ellisonbg ellisonbg Adding nbformat subpackage. 68687d2
@ellisonbg ellisonbg Notebook now uses tab for autocompletion. 4b71ab3
@ellisonbg ellisonbg Fixes to terminal mode execution (ctrl-enter). c88117a
ellisonbg added some commits Aug 15, 2011
@ellisonbg ellisonbg Adding additional whitespace at botton of notebook for TAB comp. 95590c1
@ellisonbg ellisonbg Code cell gets focused after "To Code" is triggered. 54eeb77
@ellisonbg ellisonbg Export works with filenames having spaces.
* The fix was to put the filename in double quotes in the
  Content-Disposition header.
* Export As/Clear All have been renamed to Export and Clr All
@ellisonbg ellisonbg I like ClearAll better than Clr All. 50206b2
@ellisonbg ellisonbg Pager is not activated if the pager text is empty.
* Things like asdf.*? used to open the pager even though there
  was nothing to show.
@ellisonbg ellisonbg Date is properly removed from JSON reply before WebSocket forward.
* Both the header and parent_header have a date field that cannot
  be json serialized. This field is just removed for now, but
  in the future, we will covert the date to a ISO8601 field.
* Better error handling around this code has also been added
  to prevent the server from crashing due to malformed messages.
@ellisonbg ellisonbg Kernel/notebook mapping is removed when a kernel dies.
* Previously, when a kernel died due to an external cause, the
  notebook/kernel mapping was not removed, so the kernel would
  be resused even though it was dead.
* The heartbeat now properly removes the notebook/kernel mapping.
@ellisonbg ellisonbg WebSocket url is now passed to browser when a kernel is started. 856b17f
@ellisonbg ellisonbg Removing old CodeMirror version. 56475d6
IPython member

Just a note for anyone interested in helping us with this PR: all feedback is welcome, but obviously on something of this size we're not going to do a line-by-line nitpick. Furthermore, the functionality developed here is so absolutely essential that there's no question this is going in, even though we are fully aware it needs more tuning.

But if anyone is interested in participating in the review and you spot any problems, by all means point them out so we can fix things before it goes into trunk and gets more users. Bonus points if you help us out by actually installing it and running it. You simply need to have at least tornado 2.0 installed, and if you are using Firefox, turn on websockets by going to the about:config page, filtering on 'websocket' and turning both options to true by double clicking on them. Once you do this, you can run it via

ipython notebook --pylab=inline

or you can omit the --pylab support if you don't want plots. pylab is only used for figures, the notebook works fine without numpy/matplotlib.

@fperez fperez and 1 other commented on an outdated diff Aug 16, 2011
@@ -43,6 +43,14 @@ sys.path.append(os.path.join(os.path.dirname(__file__), "extensions"))
from .config.loader import Config
from .core import release
from .core.application import Application
+# Todo: Should these be imported here? We need to rethink what is imported in
+# this module.
+#from .core.display import (
+# display, display_pretty, display_html, display_latex,
+# display_png, display_jpeg, display_svg, display_json,
+# display_javascript, HTML, SVG, Math, Image, JSON,
+# Javascript, Pretty
fperez Aug 16, 2011 IPython member

I suggest that instead, we have a top-level 'display' module that simply imports the things we want to expose (possibly including core and lib).

In general, I think we should follow a pattern where things meant for public use should be at most one module deep:

from import bar, baz

even if these top-level modules are mostly just pass-throughs that load from deeper parts of the hierarchy. But these will be the ones we consider public, and things deeper in are more subject to change. That way we only have to explain to people in normal docs and instructions these top-level namespaces, leaving the rest as 'an implementation detail'.

ellisonbg Aug 16, 2011 IPython member

Or do you think all of the user facing stuff (not just the display stuff) should go into a single module? Right now there is IPython.embed and we will definitely have more and more user facing code.

fperez Aug 16, 2011 IPython member

How about we just remove these for now from the merge, since the organization of the public api is really a separate issue from the notebook code, and we make sure that in the next month or two when the dust on the notebook settles, we set up a time to discuss the public api with a clear head. I don't think there's major hurry in getting that right, so as long as we do it before too long (say a few months, in the 0.12-0.13 timeframe), we'll be OK. And we can calmly review all the pieces we want exposed, and think about the cleanest way to do so...

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 1 other commented on an outdated diff Aug 16, 2011
-def display_latex(*objs):
+ Parameters
+ ----------
+ objs : tuple of objects
+ The Python objects to display, or if raw=True raw JPEG data to
+ display.
+ raw : bool
+ Are the data objects raw data or Python objects that need to be
+ formatted before display? [default: False]
+ """
+ raw = kwargs.pop('raw',False)
+ if raw:
+ for obj in objs:
+ publish_png(obj)
fperez Aug 16, 2011 IPython member

Should this really be png, or was it meant to be jpeg?

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 1 other commented on an outdated diff Aug 16, 2011
@@ -56,7 +56,7 @@ class DisplayPublisher(Configurable):
Any metadata for the data.
- if not isinstance(source, str):
+ if not isinstance(source, (str,unicode)):
fperez Aug 16, 2011 IPython member

This should use instead

isinstance(source, basestring)

which covers all the cases and is py3-compatible out of the box.

ellisonbg Aug 16, 2011 IPython member

Pushing a fix.

@ellisonbg ellisonbg Fixing two minor things for review.
* Using basestring for str, unicode test.
* png->jpeg in display_jpeg.
@fperez fperez and 1 other commented on an outdated diff Aug 16, 2011
@@ -48,6 +48,7 @@ from IPython.core.error import UsageError
from IPython.core.fakemodule import FakeModule
from IPython.core.profiledir import ProfileDir
from IPython.core.macro import Macro
+from IPython.core import magic_arguments
from IPython.core import page
fperez Aug 16, 2011 IPython member

These two lines can probably just be

from IPython.core import magic_arguments, page

so we don't accumulate extra unnecessary separate import lines.

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 2 others commented on an outdated diff Aug 16, 2011
@@ -193,6 +193,15 @@ class ProfileCreate(BaseIPythonApplication):
+ try:
+ from IPython.frontend.html.notebook.notebookapp import IPythonNotebookApp
+ except Exception:
+ # this should be ImportError, but under weird circumstances
+ # this might be an AttributeError, or possibly others
+ # in any case, nothing should cause the profile creation to crash.
+ pass
fperez Aug 16, 2011 IPython member

Which are reasonable reasons to get errors in this import? Just missing dependencies? Shouldn't we at least throw up a warning with the exception info if there's an error, so that users are made aware of a problem? Or at least throw the warning in the non-ImportError cases, if ImportError is expected to happen somewhat normally...

minrk Aug 16, 2011 IPython member

This is copied from the qt code, which could raise a variety of errors, depending on PySide/PyQt versions, etc., and there's nothing at all that should prevent profile creation. It is unlikely that the notebook app will raise anything other than ImportError, but always possible. A warning on non-ImportErrors would be appropriate, or just a debug or info-level message in every circumstance.

ellisonbg Aug 17, 2011 IPython member

I am passing on ImportError and log.debug(exec_info=True) on anything else now.

@fperez fperez commented on the diff Aug 16, 2011
@@ -48,6 +48,14 @@ def print_png(o):
png = latex_to_png(s)
return png
+def print_latex(o):
+ """A function to generate the latex representation of sympy expressions."""
+ s = latex(o, mode='equation', itex=True)
+ s = s.replace('\\dag','\\dagger')
+ return s
fperez Aug 16, 2011 IPython member

To avoid extra unnecessary assignments, the whole function can be written as:

return latex(o, mode='equation', itex=True).replace('\\dag','\\dagger')

I know one-liners can get unreadable, but in this case it doesn't seem that bad to me and it saves some time and memory (and I actually find the short form very readable). Not a big deal though, I'll leave the final decision up to you.

ellisonbg Aug 17, 2011 IPython member

I am going to leave this as it is extremely likely we will discover additional transforms that need to happen on the latex.

fperez Aug 17, 2011 IPython member

no prob.

IPython member

Note: the help links at the bottom aren't working for me now, and I'm pretty sure a few days ago I had tried them and they did work. You may want to have a look...

IPython member

Can you add the favicon from (, so tornado doesn't constantly print favicon warnings?

IPython member

Brian, note that right now, the test suite fails with:

ERROR: test_session_manager (IPython.frontend.html.notebook.tests.test_kernelsession.TestKernelManager)
Traceback (most recent call last):
  File "/home/fperez/usr/lib/python2.6/site-packages/IPython/frontend/html/notebook/tests/", line 30, in test_session_manager
    sm = km.create_session_manager(kid)
AttributeError: 'KernelManager' object has no attribute 'create_session_manager'

Ran 14 tests in 0.205s
@fperez fperez and 2 others commented on an outdated diff Aug 17, 2011
+ """Return the WebSocket URL for this server."""
+ if self.certfile:
+ prefix = u'wss://'
+ else:
+ prefix = u'ws://'
+ return prefix + self.ws_hostname + u':' + unicode(self.port)
+ def parse_command_line(self, argv=None):
+ super(IPythonNotebookApp, self).parse_command_line(argv)
+ if argv is None:
+ argv = sys.argv[1:]
+ self.kernel_argv = list(argv) # copy
+ # kernel should inherit default config file from frontend
+ self.kernel_argv.append("--KernelApp.parent_appname='%s'"
+ # scrub frontend-specific flags
fperez Aug 17, 2011 IPython member

This logic should be slightly changed: list.remove(x) is a slow, O(n) operation.

Instead lines 193-199 should be replaced with:

# scrub frontend-specific flags
nb_flags = set(notebook_flags) # O(1) checks
self.kernel_argv = [a for a in argv if not(a.startswith('-') and a.lstrip('-') in nb_flags)]
# kernel should inherit default config file from frontend
minrk Aug 17, 2011 IPython member

Don't forget to make a similar update on the aliases section as well, since your proposal only handles half of it. I presume this code is copied from my block in the qtconsole app, so the inefficient code is probably mine.

Also, remember that this is argv, so we are talking about an O(n) operation on a list usually of length 2, and maybe up to 10 in extreme cases, with a theoretical upper bound of every IPython configurable, which is O(100). And list.remove can only be called at most once for each notebook flag, which is a very small number.

fperez Aug 17, 2011 IPython member

Yes, I know these things are small, but I just like to use the right approaches when feasible (esp. when the right approach doesn't have a larger cost than the simpler one nor is it more obscure to read). I actually find the alternate code more readable than the original, FWIW :)

ellisonbg Aug 17, 2011 IPython member

With your approach the filtering of both the flags and aliases has to be done at the same time, which makes the list comprehension incomprehensible. I am leaving the same. Also, as Min said, there is 0 performance reason to make this change when O(n) == O(10).

fperez Aug 17, 2011 IPython member

no problem. My definition of readable list comprehensions often differs from that of most people ;)

IPython member

When printing, we need some kind of word wrap solution. Right now if you print a long list, you simply get a very, very wide page with a huge horizontal scrollbar. If this isn't an easy fix now, let me know and we'll just open an issue for it.

@fperez fperez and 1 other commented on an outdated diff Aug 17, 2011
+ self, self.kernel_manager, self.notebook_manager, self.log
+ )
+ if self.certfile:
+ ssl_options = dict(certfile=self.certfile)
+ if self.keyfile:
+ ssl_options['keyfile'] = self.keyfile
+ else:
+ ssl_options = None
+ self.http_server = httpserver.HTTPServer(self.web_app, ssl_options=ssl_options)
+ if ssl_options is None and not self.ip:
+ self.log.critical('WARNING: the notebook server is listening on all IP addresses '
+ 'but not using any encryption or authentication. This is highly '
+ 'insecure and not recommended.')
+ for i in range(10):
+ try:
+ port = self.port + i
fperez Aug 17, 2011 IPython member

I would rewrite this logic as:

        from random import randint
        n = 50 # max number of attempts, keep reasonably large
        for port in [self.port] + [self.port + randint(-2*n, 2*n) for i in range(n)]:
                self.http_server.listen(port, self.ip)
            except socket.error, e:
                if e.errno != errno.EADDRINUSE:
      'The port %i is already in use, trying another random port.' % port)
                self.port = port

In cluster-type environments and other shared resources, it will be very easy to find the range of ports 8888-8898 to be occupied. This approach tries 50 random ports centered around 8888, which is far more likely to succeed after one or two tries, and will in general be faster for users than manually walking the list every time.

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 1 other commented on an outdated diff Aug 17, 2011
+ allowed_formats = List([u'json',u'xml',u'py'])
+ # Map notebook_ids to notebook names
+ mapping = Dict()
+ # Map notebook names to notebook_ids
+ rev_mapping = Dict()
+ def list_notebooks(self):
+ """List all notebooks in the notebook dir.
+ This returns a list of dicts of the form::
+ dict(notebook_id=notebook,name=name)
+ """
+ names = os.listdir(self.notebook_dir)
+ names = [name.split(u'.')[0] \
fperez Aug 17, 2011 IPython member

final backslash not needed here (the open list makes it ok to continue without backslash)

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 1 other commented on an outdated diff Aug 17, 2011
+ return notebook_id
+ def delete_notebook_id(self, notebook_id):
+ """Delete a notebook's id only. This doesn't delete the actual notebook."""
+ name = self.mapping[notebook_id]
+ del self.mapping[notebook_id]
+ del self.rev_mapping[name]
+ def notebook_exists(self, notebook_id):
+ """Does a notebook exist?"""
+ if notebook_id not in self.mapping:
+ return False
+ path = self.get_path_by_name(self.mapping[notebook_id])
+ if not os.path.isfile(path):
+ return False
+ return True
fperez Aug 17, 2011 IPython member

The last three lines should just be

return os.path.isfile(path)

Both clearer and faster.

ellisonbg Aug 17, 2011 IPython member


@fperez fperez and 1 other commented on an outdated diff Aug 17, 2011
+ if not os.path.isfile(path):
+ raise web.HTTPError(404)
+ info = os.stat(path)
+ last_modified = datetime.datetime.utcfromtimestamp(info.st_mtime)
+ try:
+ with open(path,'r') as f:
+ s =
+ try:
+ # v2 and later have xml in the .ipynb files.
+ nb = current.reads(s, 'xml')
+ except:
+ # v1 had json in the .ipynb files.
+ nb = current.reads(s, 'json')
+ # v1 notebooks don't have a name field, so use the filename.
+ = os.path.split(path)[-1].split(u'.')[0]
+ except:
fperez Aug 17, 2011 IPython member

I'm a little worried that any python error here will map to a 404, because it will make debugging problems in this kind of code hard, no? Would it be reasonable to have some kind of debug flag in which the 404 page is returned but with traceback info in it of some kind? I know the cgitb module from the stdlib helps format tracebacks in html, so perhaps it could be used here.

ellisonbg Aug 17, 2011 IPython member

I will open an issue for this. I think we can send back JSON data describing the error. I don't think we want to send back an actual traceback, but some sort of message that the browser can use to programatically determine the source of the error. I am not too worried about it because I have used HTTP error codes that are themselves almost completely descriptive.

@fperez fperez commented on the diff Aug 17, 2011
+ # v1 had json in the .ipynb files.
+ nb = current.reads(s, 'json')
+ # v1 notebooks don't have a name field, so use the filename.
+ = os.path.split(path)[-1].split(u'.')[0]
+ except:
+ raise web.HTTPError(404)
+ return last_modified, nb
+ def save_new_notebook(self, data, name=None, format=u'json'):
+ """Save a new notebook and return its notebook_id.
+ If a name is passed in, it overrides any values in the notebook data
+ and the value in the data is updated to use that value.
+ """
+ if format not in self.allowed_formats:
+ raise web.HTTPError(415)
fperez Aug 17, 2011 IPython member

Again, regarding debuggability: is it possible to include some information about the error? Something like raise web.HTTPError(415, "invalid format %s supplied" % format)?

ellisonbg Aug 17, 2011 IPython member

In this case 415 means "Unsupported Media Type", which is basically the information we would return. I have been quite careful to return error codes that are specific.

fperez Aug 17, 2011 IPython member

ok, thanks for the info

@fperez fperez commented on an outdated diff Aug 17, 2011
+ If a name is passed in, it overrides any values in the notebook data
+ and the value in the data is updated to use that value.
+ """
+ if format not in self.allowed_formats:
+ raise web.HTTPError(415)
+ try:
+ nb = current.reads(data, format)
+ except:
+ if format == u'xml':
+ # v1 notebooks might come in with a format='xml' but be json.
+ try:
+ nb = current.reads(data, u'json')
+ except:
+ raise web.HTTPError(400)
fperez Aug 17, 2011 IPython member

Same comment as above regarding extra info in these exceptions, and the same will apply wherever there's one of these (I won't add it on every line, but the idea stands).

@fperez fperez commented on the diff Aug 17, 2011
+body {
+ background-color: white;
+ /* This makes sure that the body covers the entire window and needs to
+ be in a different element than the display: box in wrapper below */
+ position: absolute;
+ left: 0px;
+ right: 0px;
+ top: 0px;
+ bottom: 0px;
+ overflow: hidden;
+div#header {
+ /* Initially hidden to prevent FLOUC */
fperez Aug 17, 2011 IPython member

just curious, what is FLOUC??

ellisonbg Aug 17, 2011 IPython member

Flash of Unformatted Content.

fperez Aug 17, 2011 IPython member

thanks. I'm learning :)

@fperez fperez commented on the diff Aug 17, 2011
@@ -0,0 +1,42 @@
+ * Auto Grow Textarea Plugin
+ * by Jevin 5/11/2010
+ *
+ *
+ * Modified by Rob G (aka Fudgey/Mottie)
+ * - Converted into a plugin
+ * - Added ability to calculate approximate # cols when textarea is set to 100%
+ *
+ * Simplified by Brian Granger on 5/2/2011
fperez Aug 17, 2011 IPython member

Just curious, is this already in upstream or are these your local changes? I just want to make sure that if we have local changes to upstream libraries, it's easy to update upstream versions and still know what our changes were to reapply them if needed.

ellisonbg Aug 17, 2011 IPython member

We are not using this anymore, but I don't think these changes should go back upstream. The logic that was in there originally was fine, but we just needed different logic because of our particular needs.

fperez Aug 17, 2011 IPython member

So now, we could use 100% upstream? If that's the case, then we should update our tree to be the pure upstream one, so it's clear there are no extra changes of our making.

ellisonbg via email Aug 17, 2011 IPython member
fperez Aug 17, 2011 IPython member

ok, sounds good.


Is a naked print a good idea here? If so, then at least we should do

from __future__ import print_function

at the top and use print() instead, so the 2to3 changes are minimized.

IPython member

Since it's one of the few things 2to3 handles well, making the from __future__ import print_function change manually, really has zero benefits as far as I know, and is primarily a cause for annoyance.

IPython member

I guess I'm just getting used to using print as a function everywhere in anticipation of the transition to 3, so it doesn't bother me much. But I agree that it's not a big deal, and I have no problem if it stays as-is.

IPython member

In ipython / IPython / nbformat / v2 / I think it would be good to have a little explanation of the format in the top-level docstring, that provides an overview of how the format is strucutred. It will help anyone diving into the code to have a big picture understanding. It doesn't need to be much, just a paragraph or two.

IPython member

The file ipython / IPython / nbformat / v2 / (the diff was so big that github stopped showing it for the last files, so I have to do these as standalone comments now) should have at least a docstring indicating what it is.

IPython member

Do we still want to have a standalone ipython-notebook script in scripts?

IPython member

OK, I'm done. I think all the above are pretty easy to do/fix, and then as far as I'm concerned we can merge!!! This was a spectacular job, I couldn't be more excited about this... Hats off, Brian, you really rock.

IPython member

Note: there was a commit that contained a test_hist.sqlite in notebook/tests file which shouldn't have been committed. That commit should be purged and we can just rebase the whole thing so the final version when merged doesn't contain that file.

IPython member
IPython member
ellisonbg added some commits Aug 17, 2011
@ellisonbg ellisonbg Don't scroll to bottom when last cell is selected. 2c580e7
@ellisonbg ellisonbg More review changes.
* Favicon.ico is served.
* Test suit now passes.
* Help links work for for me.
* Other changes made to address inline comments.
* The printing of long lines is an extremely subtle issue and I will open an issue for it.
* is completely gone so the naked print is not an issue.
* ipython-notebook removed from scripts.
* Updated copyright and authors of files.
* Fixed missing docstrings in IPython.nbformat.
IPython member

OK, I am done with addressing the review comment. It would be good if everyone could run it through its paces one more time though.

ellisonbg and others added some commits Aug 17, 2011
@ellisonbg ellisonbg Finish removing ipython-notebook. 2e45b3a
@ellisonbg ellisonbg Adding code to handle MozWebSocket for FF 6. bf61e42
@ellisonbg ellisonbg Better WebSocket detection added. 719e65a
@ellisonbg ellisonbg Better alert message if no WebSockets are detected. 486e84f
@ellisonbg ellisonbg Removed HTMLCell from UI and added better placeholder logic. eb895bf
@stefanv stefanv Refactor static printing. 968e3fe
@ellisonbg ellisonbg Modifying CodeMirror focus hack to work better.
I had removed a text area focus event earlier today and that
broke the . key. We are still having problems with CodeMirror
elements gaining focus, but this seems to fix the issue.
@ellisonbg ellisonbg Fixing parallel options pricing example. 7777bdd
@ellisonbg ellisonbg Merge branch 'htmlnotebook_publish' of…
…thon into stefanv-htmlnotebook_publish
@ellisonbg ellisonbg Changing notebook uuid algorithm to preserver across sessions. f4c8372
@stefanv stefanv Allow period characters in notebook names. a2b246f
@stefanv stefanv Move glob to global level import. 0e2ee7f
@ellisonbg ellisonbg Adding page break logic to the print css.
* I have added page-break-inside logic to div.input and
* Fixed a bug in CodeCell that was putting the output_area class
  on the wrong div.
@ellisonbg ellisonbg Merge branch 'stefanv-htmlnotebook_publish' into htmlnotebook d33fac5
@ellisonbg ellisonbg Merge branch 'htmlnotebook_list_notebooks' of…
…anv/ipython into stefanv-htmlnotebook_list_notebooks
@ellisonbg ellisonbg Double clicking on the end space will insert a new cell. cb5364f
@ellisonbg ellisonbg Better tabindex support. bdb89a7
@ellisonbg ellisonbg Save button becomes Rename when the notebook name changes. 507621d
@ellisonbg ellisonbg Help links work on Firefox. 1e2b197
@ellisonbg ellisonbg Fixing logic for rename behavior. 6d3001f
@ellisonbg ellisonbg Making JSON the default .ipynb format. cc19c96
@ellisonbg ellisonbg Converting notebooks to JSON format. 1cfbe5d
@ellisonbg ellisonbg Notebook upload handles filenames with periods. 33e3025
@minrk minrk commented on the diff Aug 19, 2011
+ return last_modified, name, data
+ def get_notebook_object(self, notebook_id):
+ """Get the NotebookNode representation of a notebook by notebook_id."""
+ path = self.find_path(notebook_id)
+ if not os.path.isfile(path):
+ raise web.HTTPError(404)
+ info = os.stat(path)
+ last_modified = datetime.datetime.utcfromtimestamp(info.st_mtime)
+ with open(path,'r') as f:
+ s =
+ try:
+ # v1 and v2 and json in the .ipynb files.
+ nb = current.reads(s, u'json')
+ except:
+ raise web.HTTPError(404)
minrk Aug 19, 2011 IPython member

We need a lot more feedback than a 404 here, especially since this isn't even the real page, so users will get exactly no feedback.

The frontend just sticks at 'loading...', and if they enter a name and save, the original corrupt (or xml) notebook is simply erased.

ellisonbg and others added some commits Aug 19, 2011
@ellisonbg ellisonbg Implemented metadata for notebook format. d919e2e
@ellisonbg ellisonbg Changing CodeMirror-scroll to overflow-y: hidden.
* It seems that our old setting of visible was causing scroll bars
  to appear if font sizes change in Chrome.
* Also changed json -> xml in Download UI.
@ellisonbg ellisonbg Cell collapse/expand is not called "Toggle". 8013d52
@ellisonbg ellisonbg Reorganize the L panel buttons. b1af148
@ellisonbg ellisonbg Fixing bug in new metadata implementation. bc7865e
@ellisonbg ellisonbg All output types are not indented. 9748902
@ellisonbg ellisonbg Adjusting width of prompt area. 54111d3
@fperez fperez Right-align prompts to remove spurious whitespace. b5134ae
@ellisonbg ellisonbg Adding keyboard shortcuts. d11e8eb
@ellisonbg ellisonbg Adding keyboard shortcut help dialog. 7e2eee1
@ellisonbg ellisonbg Fixing console.log messages related to keyboard shortcuts. 4a3930a
@ellisonbg ellisonbg Changing prev/next keyboard shortcut to use p/n. 14ae374
@ellisonbg ellisonbg Fixing XML notebook reader. 5dc0439
@stefanv stefanv Align colons in help dialog. c5f13bc
@ellisonbg ellisonbg Merge pull request #713 from stefanv/htmlnotebook_help_dialog
Align colons in html notebook help dialog
@minrk minrk Add utility function for installing mathjax for offline use
from IPython.external.mathjax import install_mathjax


* add mathjax destination to gitignore
@minrk minrk update notebook template to prefer offline mathjax 89aca0b
@stefanv stefanv Add code highlighting to markdown cells. d6a97e9
@stefanv stefanv Add prettify license. e591917
@stefanv stefanv Try to match CodeMirror theme. 88fabb9
@fperez fperez Merge pull request #717 from stefanv/htmlnotebook_highlight_markdown
Add source highlighting to markdown snippets, with a theme matching the CodeMirror one we use.

This only highlights source code in blocks that are indented 4 spaces in markdown cells, leaving <pre> blocks alone.  If highlight is desired in <pre> blocks, a further <code> block must be created.

The visual theme matches the one used for CodeMirror as much as possible.
@fperez fperez Fix above/below keybinding mismatch and rename api to use above/below 9600e07
@minrk minrk underline keyboard shortcut letter on buttons de11baf
@ellisonbg ellisonbg Merge branch 'mathjax' of into minrk…
@ellisonbg ellisonbg Adding information about MathJax to notebook install docs. 0bfb0fb
@ellisonbg ellisonbg Temporary fix for placeholder related CM bug. 876e689
@ellisonbg ellisonbg merged commit 876e689 into master Aug 23, 2011

Those 2 bindings seem not to be used. The collapse/expand_pager triggerd seem to be in notebook.js. Am I wrong ? Should I remove them from here ?

IPython member

Hum, it seem that I'm wrong... I'm not sure t understand how it works then... and what's the difference between those binding and the ones in notebooks.js
Sorry for the noise.

IPython member

I don't know why we have both of them, but I did a test and confirm that both are being called when pager.js L62 is run. W can probably move all that logic to pager.js as I don't see anything notebook specific in the notebook.js version. This code was written very early on so I am not surprised it is crusty.

IPython member

I think the one in pager.js grow/shrink the pager below the separator while the one in notebook.js shrink/grow the height of the notebook area. So it kind of make sens to have function dealing with notebook in notebook..

IPython member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment