[IPython-dev] Comments about IPython.frontend, getting ready for merge of ipython-sync-frontend

[Brian Granger]

Hello all (especially Barry and Gael),

We would like to merge the ipython-sync-frontend branch into trunk
ASAP.  Fernando will be sending out an email later today describing
our timeframe for the upcoming release.  Meanwhile, I wanted to
provide some informal code review and feedback on the stuff in
IPython.frontend.  I haven't looked over the wx or cocoa specific
stuff, just the general stuff.

Overall, I am _really_ excited about this code.  Many thanks to Barry
and Gael for pushing this stuff forward and making the first go and
using the new core.  I talked to Fernando last night and both of us
are completely committed to getting this stuff into the next IPython
release, which is coming soon.  I should preface my comments by saying
that I don't think all of the comments need to be address before the
merge or the release.  Some of the comments are more long term.  Also,
Fernando is going to look over the new stuff in IPython.kernel.core (I
haven't looked at this stuff).


Here are my comments:

===============================
Comments about IPython.frontend
===============================

General
=======

We need a robust way for modules in the frontend to handle interfaces
if zope.interface is not installed.  Currently each module tries to
import zope.interface and then builds a mock zope.interface, like
this::

	try:
	    from zope.interface import Interface, Attribute, implements, classProvides
	except ImportError:
	    Interface = object
	    def Attribute(name, doc): pass
	    def implements(interface): pass
	    def classProvides(interface): pass

This code should probably be put into a common location, something
like frontend.zopeinterface, so that it can be reused.  But eventually
when
IPython.core is created, this should probably be moved there.

The docstrings are vague and outdated at points.  More detailed and
updated docstrings would be helpful.  Also, all docstrings should use
the epydoc+ReST format.

IPython.kernel.core should probably be moved to IPython.core.

Once Fernando's testing branch has been merged, more tests should be written.

frontendbase.py
===============

Configuration should be done using the approach in IPython.config.
See IPython.kernel.config and IPython.kernel.scripts.ipcontroller for
examples.

IFrontEnd has references to Twisted in its docstrings.  Because the
basic frontend should work without Twisted, the docstrings should be
updated to clarify how the interface plays with Twisted, even though
it is optional.  We really want to document for developers that the
base frontend is synchronous and doesn't require Twisted, but that the
interface has a particular design that allows it to be wrapped into an
asynchronous frontend (like some methods pass through the result).

The design of the frontend seems good though.  We might eventually
want to think about using the notification stuff in the frontend
though.

Why does FrontEndBase not have a complete method?  Other frontend
classes do have this method and its seems like the base class should
have it too.  Also, there are oher methods in the interpreter that we
might want to propagate up to the frontend, like push and pull.
Thoughts?  If these additional methods go in, there should be separate
asynchronous versions that return deferreds, but these would go into
asyncfrontend.py.

asyncfrontendbase.py
====================

Because this frontend us designed for use with Twisted, I don't think
we need to protect the zope.interface and Twisted imports.  Also setting
Failure to Exception will break things as they don't have the same interface.

Design looks good though.

linefrontendbase.py
===================

This looks good as it is simply a concrete subclass of FrontEndBase.
General comments above about docstrings apply.

prefilterfrontend.py
====================

It is fantastic that we can combine ipython0 and IPython.kernel.core
code like this.  This is a great way of using the new stuff, but
getting the more complete feature set of IPython.*  But I think this
type of thing should be done in the lower-level Interpreter, rather
than the frontend.  This is for a couple of reasons:

	1. This would give all users of the Interpreter access to completion and the
	   prefilter stuff.  We really need this stuff in the parallel
computing stuff,
	   as well as non-line-oriented frontends.
	2. The Interpreter already has some of this logic and it is simply being
	   repeated.
	3. The current implementation requires that the Interpreter is in-process.
	   For connecting to a remote Interpreter, this won't work.
	
But, we might run into a problem with doing this stuff (that cool hack
that is begin used currently) in the Interpreter.  The reason is that
the Interpreter is block based, whereas the prefilter stuff in IPython
is only line based.  Thus, doing this refactoring might be a little
more involved.  But it is the eventual design we want to go for.  This
stuff probably won't be done before the merge. We can work on this at
SciPy.


Great work everyone!!!

Brian