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

[Gael Varoquaux]

Forgot to Cc the mailing list.

----- Forwarded message from Gael Varoquaux <gael.varoquaux at normalesup.org> -----

Date: Mon, 11 Aug 2008 20:43:43 +0200
From: Gael Varoquaux <gael.varoquaux at normalesup.org>
To: Fernando Perez <fperez.net at gmail.com>
Subject: Re:  [IPython-dev] Comments about IPython.frontend, getting ready
	for merge of ipython-sync-frontend

On Fri, Aug 08, 2008 at 02:05:54AM -0700, Fernando Perez wrote:

> fd_redirector
> =============

> I imagine this is the part you've been struggling with, from your comments. 

Partly, but that's only a tiny fraction of the tip of the iceberg. The
hardest part was embedding all this in a GUI loop and try to stay
interestive without threads.

> We certainly need this to gracefully handle i/o under GUIs.  Docstrings
> and tests are critically needed though, and the docstrings should
> conform to the reST api for sphinx.

Docstring and tests are now there. All this should work under windows,
including the tests.

> All attributes should be declared in the constructor, even if they are set to
> None with a comment of what they do and that their true value is given later
> (such as in start() ).  The fact that Python allows us to create attributes at
> any point doesn't mean we should do it in normal code, its a feature whose
> valid use cases are in decorating/annotating external objects.  But classes we
> write should declare their attribute API completely in the constructor, for
> readability/comprehensibility reasons.

> Is it necessary to call stop() in getvalue()?

Pretty much. I think I need to close the file descriptor, and redo the
duplication, so I pretty much need to implement the stop method in the
getvalue.

> file_like.py
> ============

> - writelines should read simply "map(self.write,lines)".  Faster, cleaner,
> shorter.

Done.

> - why are the other methods implemented as 'pass'?  Wouldn't it be better not
>   to have them?  Making them pass means they return None, which can lead to
>   them being used by other code which will then strangely break when None is
>   the wrong thing to get.

I have to implement them to satisfy the file-interface. I have documented
this requirement in the code, now.

> output_trap.py
> ==============

> This one implements a smaller set of functionality than the original
> OutputTrap.  Why?  Do we need to merge the two?  Right now we have in
> core.shell uses of the old module and in core.interpreter, of the new, so
> likely some sort of merge will be needed.

Hum, I am not too sure I am following. I haven't created a second
implementation of something, AFAIK.


> redirector_output_trap.py
> =========================

> No docstrings on constructor.

Fixed.

> sync_traceback_trap.py
> ======================

> No docstrings on constructor. The top-level docstring could be a bit more
> verbose.

I improved this, but I must admit I don't fully understand the way
traceback_trap is supposed to work (no docstrings), so I cannot fully
document this object?

I now feel that the branch is in good-enough shape to be merged in.
Obviously there still is some work to be done (more test for a better
coverage, althought the GUI frontends are pretty much impossible to
test). I however feel that we should merge ASAP, and that I will continue
to work on this in the trunk.

I would appreciate another quick review to check that everything is in
order, and for more feedback.

Gaël


----- End forwarded message -----