[IPython-dev] Code review (mostly for Fernando)

Brian Granger ellisonbg at gmail.com
Sat Sep 18 12:10:46 EDT 2010


Fernando,

I just looked through everything and it looks great. Thanks for doing
this.  Things are looking really good.

Brian

On Sat, Sep 18, 2010 at 3:13 AM, Fernando Perez <fperez.net at gmail.com> wrote:
> Hey,
>
> I've just finished up the main cleanups post-review, thanks again!
>
> In the process I ended up implementing call signature in the calltips,
> along with at least a few unittests for the object inspector (going
> with the idea of trying to add unittests to any new code as much as
> possible).  Now that we have calltips, I was really missing the
> signature in them :)
>
> Evan had mentioned adding more sophisticated calltips that would carry
> syntax highlighting of the current argument being typed; all the info
> is in there and we can extend to that later. For now I'm just
> formatting the signature as a plain string, and if someone can
> contribute the fancier version later, it's just a matter of changing
> one flag to prevent plain string conversion and doing something
> better.  But even the plain string version is very useful, I think.
>
> On Tue, Sep 14, 2010 at 10:59 PM, Fernando Perez
> <Fernando.Perez at berkeley.edu> wrote:
>
>>> Does the kernel know anything about cell mode?  If so, we need to go over this
>>> more carefully. Added later: I now see that the kernel does know about cell
>>> mode. We should talk more about this.
>>
>> Yes, what we need to do is rationalize the number of 'run*' entry
>> points we have.  Those have accreted over time, starting with the old,
>> convoluted and unnecessary approach that we inherited from code.py
>> (push()->runsource()->runcode()).
>
> [...]
>
> I documented the execution semantics a lot more:
>
> http://ipython.scipy.org/doc/nightly/html/development/messaging.html#execute
>
> We still have more work to do on this front, as discussed.  I'll try
> to crack the off-by-one prompt bug and organize the core execution
> loop better this weekend.
>
>> Yes, paste is not ideal and I don't like it either.  I was thinking
>> 'pastefig' earlier today: it's pretty explicit and matches the
>> sound/feel of 'savefig', already in mpl.  How does that sound?
>
> Done, it's now pastefig()
>
>>> Let's rename "instant_death" to "now" everywhere as people are used to this
>>> word in this context (sudo shutdown -h now)
>>
>> Sounds good, I'll do that in a quick commit now (no pun intended :).
>> I wasn't very happy with that name either, but couldn't think of a
>> better one.
>
> Done.
>
>>> The initialization of the input_splitter in init_instance_attrs should be
>>> moved to somewhere else. Basically, init_instance_attrs should eventually be
>>> retired and anything that is traited should not be in there.  You can have
>>> traits autocreate the input splitter by doing::
>>>
>>> input_splitter =
>>> Instance('IPython.core.inputsplitter.IPythonInputSplitter',(),{})
>>
>> Ah, OK, cool.
>
> Done.
>
>>> http://github.com/ipython/ipython/commit/e7e389c1052ceda15656f84edf8cff41f3640992
>>>
>>> I know this is not nice to say, but I think we should use a different data
>>> structure than a NamedTuple or dict here. Basically, we want the message
>>> protocol to use universal data structures. The closest thing to a NamedTuple
>>> is a list of tuples:
>>>
>>> data = [('type_name',type_name),('base_class',base_class), etc.]
>>>
>>> This will be both ordered and easy for both Python and Javascript to
>>> parse and print.  It will also be easy for Python frontends to put the data
>>> back into a NamedTuple if needed.  But the wire protocol should be universal.
>>> If we just send the dict, Javascript clients will loose the ordering info.
>>
>> Totally agreed, no worries.  Right now only a python client could
>> reconstruct the order by importing the fields list, by switching to a
>> list-of-pairs we'll make it language-agnostic.
>
> I removed the namedtuple but left it as a dict for now.  The
> underlying data structure was a dict and that goes fine over json, so
> there's no problem with that.  Even for non-python clients, it may be
> more convenient to just leave the json dict alone and ship (once) the
> list of ordered names. Since that may turn out to be the cleaner
> solution in the long run, and leaving the dicts alone actually made
> the *current* code we do have a fair bit simpler, I decided against
> the list-of-tuples for now.  Go with the simplest solution that works
> and only add complexity later if we do really have a use for it.
>
>>> http://github.com/ipython/ipython/commit/4e3685be1b53680b1894da6bba96141ed582b123
>>>
>>> Add blank line right before def _ofind(...)
>>
>> yup :)
>
> Done.
>
>>> http://github.com/ipython/ipython/commit/2be9133b2a3035f51932b3ebd9ac054a9b3e28ba
>>>
>>> Maybe rename get_user_variables to something that reflects we are getting
>>> the repr's of the variables?
>>
>> We have these two that return reprs:
>>
>> get_user_variables
>> eval_expressions
>
> Done as discussed: user_variables, user_expressions.  Documented and
> changed the method api to match the messaging spec.
>
> Thanks again!
>
> Cheers,
>
> f
>



-- 
Brian E. Granger, Ph.D.
Assistant Professor of Physics
Cal Poly State University, San Luis Obispo
bgranger at calpoly.edu
ellisonbg at gmail.com



More information about the IPython-dev mailing list