[IPython-dev] Comments about IPython.frontend, getting ready for merge of ipython-sync-frontend (fwd)
Mon Aug 11 13:44:31 CDT 2008
Forgot to Cc the mailing list.
----- Forwarded message from Gael Varoquaux <firstname.lastname@example.org> -----
Date: Mon, 11 Aug 2008 20:43:43 +0200
From: Gael Varoquaux <email@example.com>
To: Fernando Perez <firstname.lastname@example.org>
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:
> 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
> - writelines should read simply "map(self.write,lines)". Faster, cleaner,
> - 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.
> 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.
> No docstrings on constructor.
> No docstrings on constructor. The top-level docstring could be a bit more
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.
----- End forwarded message -----
More information about the IPython-dev