add BlockingKernelClient.execute_interactive #185

minrk · 2016-08-11T09:35:16Z

Redisplays output, returns reply.

Can be used in #184

Closes #176

minrk · 2016-08-11T09:40:57Z

I didn't really like calling this run, since I think this is how execute should behave on the blocking client, but that would be backward-incompatible.

@takluyver what do you think about BlockingClient adding a reply=True kwarg to trigger waiting for and returning a reply on every req/rep message? That way, the regular methods could be used without needing to come up with synonyms.

Captures output, returns reply

waits for and returns reply instead of msg_id

minrk · 2016-08-11T11:46:38Z

I like the reply=True better, since it's not creating new methods, just completing the loop started by the existing methods, so I went ahead and updated this PR.

None is not valid in this case

dsblank · 2016-08-11T12:46:31Z

Thanks, @minrk ... this works pretty well! One thing I am having trouble with is input() should that be able to work?

minrk · 2016-08-11T12:49:42Z

@dsblank not the way I've implemented it, I didn't include support for stdin requests, but I can.

minrk · 2016-08-11T13:11:30Z

@dsblank it handles input now.

takluyver · 2016-08-11T13:29:43Z

jupyter_client/blocking/client.py

+ Parameters
+ ----------
+ """)
+ parts.append("""


I'm a bit leary of the docstring munging, because I don't think we'll remember it when editing docstrings - e.g. if we add a note below 'returns', it will be chopped off. Not a big deal, though.

Yeah, me, too. I couldn't think of a better way, though. If you have a less gross idea, I'm all for it.

The main reason I did this is that I can't see a good way to get a nice signature for the wrapped functions. If I had a good signature, I'd be happier with a simpler "See KernelClient.method for more details...". It's easy to inherit the wrapped method's signature exactly, but I didn't see a good way to do that and add the reply, timeout arguments.

If we're willing to depend on (or bundle) a backported version of the Py3 inspect.signature machinery for Py2, it should be relatively easy to add a couple of parameters to a signature and set the f.__signature__ attribute. IIRC, IPython's inspection will use that information even on Python 2, which is good enough for me.

dsblank · 2016-08-11T13:34:15Z

Works great!

takluyver · 2016-08-11T13:36:09Z

jupyter_client/blocking/client.py

+ stream.write(content['text'])
+ elif msg_type in ('display_data', 'execute_result', 'error'):
+ if in_kernel:
+ session.send(socket, msg_type, content, parent=parent_header)


I'm a bit uncomfortable about this, because it's making a general method aware of a certain specific context in which it may be running.

Fair enough. What would you prefer? What I really want is for rich Jupyter display methods to be fully redisplayed when run in a an IPython kernel, and for plaintext output to be shown in a simple terminal way when that's the case.

Do you think these should be different methods, or explicit user-provided flags?

I think I would actually make this back to being a separate method. I think the kw arg works for the general reply=True to wait for the response, but this feels like another level on top of what the kernel client methods usually do. Maybe execute_interactive()?

Sounds good to me. I'll leave execute(reply=True) matching the other methods, without touching output.

dsblank · 2016-08-11T13:50:32Z

@minrk This issue is also a problem for non-ipython kernels. That is, we have to monkeypatch IPython.display.display in order to get rich display. If a kernel had a special method for this, then in_kernel and non-ipython kernels could both call it.

minrk · 2016-08-11T13:55:01Z

@dsblank good point. What do you folks think? Special method or argument for kernel-aware behavior?

minrk · 2016-08-11T14:01:35Z

Honestly, I do think that the default behavior in IPython should be to redisplay rich output, so detecting IPython kernels by default still seems like the right choice to me. But I take @dsblank's point that to make it more generally useful, there should be some ways to pass explicit arguments in addition to that.

dsblank · 2016-08-11T14:15:33Z

@minrk That makes sense (there has to be a default). Just remember that some languages will make this the interface to their language. As long as there is some method for dealing with the issue that is not too painful, that should work. Thanks for this!

output_hook is called with output messages, so that other clients can use this (e.g. nbconvert) Default is to redisplay, including detection of IPython kernel for rich outputs.

minrk · 2016-08-11T14:55:21Z

It is now execute_interactive. It now takes an output_hook callable that will be passed all output messages. The default is still to redisplay, including detection of IPython, but passing the hook lets the caller decide what to do with all IOPub messages. I think that should be enough that we can use this in nbconvert --execute now, as well as allow uses in any other kernel context.

takluyver · 2016-08-11T14:59:12Z

I guess it should have a stdin_hook as well, then.

Pfff, this is growing complexity rapidly. I think I'd be inclined to say it's intended as a terminal method, stick to displaying text output, and maybe offer a hook just for rich output (display_data/execute_result).

minrk · 2016-08-11T15:08:53Z

I think I'd be inclined to say it's intended as a terminal method,

I would disagree with this, since the main reason I want this is to use it for interacting with kernels from notebooks. jupyter-run is just one entry point, but not the primary one, for me.

so that input request handling can be customized

takluyver · 2016-08-11T15:21:07Z

Fair enough. I'm just disappointed that something that originally seemed quite simple has now ballooned into a fair amount of extra complexity.

minrk · 2016-08-11T15:24:46Z

Fair enough. I don't feel bad, though, as the extra complexity should also mean that it ought to be able to replace a bunch of similar code in existing projects (nbconvert, jupyter_console, dask) once this is out. So to me it's more a consolidation of code to where it belongs.

takluyver · 2016-08-11T15:39:15Z

jupyter_client/blocking/client.py

+ allow_stdin = self.allow_stdin
+ if allow_stdin and not self.stdin_channel.is_alive():
+ raise RuntimeError("stdin channel must be running to allow input")
+ msg_id = super(BlockingKernelClient, self).execute(code,


This doesn't need to be a super() call now that it's a separate method, right?

Good catch, fixed.

(annoying)

takluyver · 2016-08-11T15:42:12Z

Other than that comment, this looks OK. I've milestoned it for 5.0 as it's clearly a new feature, but I won't get upset if we want to call it 4.4.

minrk · 2016-08-11T15:43:32Z

Since 4.4 is about ready to ship, I'm happy calling it 4.5. I don't know that we have any breakages planned that would cause 5.0 in the forseeable future, though.

minrk · 2016-08-11T15:43:48Z

(If there is one, 5.0 is fine, too)

takluyver · 2016-08-11T15:45:04Z

Fair enough. I'm never entirely sure whether 4.x+1 releases are feature releases or bugfix releases.

minrk · 2016-08-11T15:57:32Z

I'm never entirely sure whether 4.x+1 releases are feature releases or bugfix releases.

We are on minor releases until there's a big and/or breaking change that prompt the major version bump. This doesn't happen often on the smaller repos, so I don't expect to have things like backport branches of ipykernel or jupyter_client or nbformat. It happens almost immediately on the big, fast repos like IPython and notebook, but there aren't many of those.

minrk · 2016-08-11T16:18:26Z

I made jupyter/nbconvert#360 to see if this works over there. It's almost enough, but there are two features over there that I haven't implemented here:

flushing output on its own, e.g. calling again after timeout (perhaps this should be split into a dedicated method)
a separate timeout that just checks the time between outputs, which better detects 'stuck' kernels vs long-running code that keeps producing output

minrk · 2016-08-11T16:18:44Z

I can save those for another PR, though

ivan-krukov · 2016-09-07T20:40:53Z

Is there a way that this can be used to echo input and traceback in a remote client?

#example.py
from jupyter_client import find_connection_file, BlockingKernelClient
kc = BlockingKernelClient(connection_file = find_connection_file())
kc.load_connection_file()
kc.execute_interactive('a=2')

Maybe I am missing the point, but running python example.py with an active jupyter console does not echo any output. It does produce produce a with a value of 2 in the target kernel, though.
I am more interested in tracebacks (via output_hook), but this is a starting point.

minrk · 2016-09-08T13:48:25Z

@ivan-krukov the code you gave has no output, so I wouldn't expect to see any. When I run something that has an exception, I do see it:

from jupyter_client import find_connection_file, BlockingKernelClient
kc = BlockingKernelClient(connection_file = find_connection_file())
kc.load_connection_file()
kc.execute_interactive('1/0')

produces:

---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)
<ipython-input-5-969594e39729> in <module>()
----> 1 a/0

ZeroDivisionError: division by zero

ivan-krukov · 2016-09-08T15:20:23Z

@minrk Thanks.
I was expecting to see the output in the console of the target kernel. I assume that can be done by posting to iopub channel (with having ZMQTerminalInteractiveShell.include_other_output=True)?

minrk · 2016-09-09T08:21:22Z

Yes, the outputs are posted to IOPub, so they will appear in the console if that flag is specified, but not immediately. Since this was originally written against readline, we could only check for outputs after each prompt, and with prompt-toolkit it looks like we are now only checking on each execution. However, because prompt-toolkit has a proper eventloop, we should be able to handle it better than before. I opened jupyter/jupyter_console#105 to track this.

minrk · 2016-09-12T09:41:12Z

Since 4.4 is out, I'll merge this soon unless people want more time to review.

minrk mentioned this pull request Aug 11, 2016

Adds jupyter-run to jupyter_client #184

Merged

minrk force-pushed the KernelClient.run branch from cb9602f to fd2bc4c Compare August 11, 2016 09:39

add BlockingKernelClient.run

fdb9356

Captures output, returns reply

minrk force-pushed the KernelClient.run branch from fd2bc4c to fdb9356 Compare August 11, 2016 09:45

add reply=True arg for all of Client req/rep methods

9967225

waits for and returns reply instead of msg_id

minrk force-pushed the KernelClient.run branch from 05ed320 to 9967225 Compare August 11, 2016 11:45

set default session, start if access=range

e72cc8f

None is not valid in this case

test with -v

42fd5b8

takluyver reviewed Aug 11, 2016
View reviewed changes

Handle stdin in execute

e3a7628

Make it execute_interactive

9687697

output_hook is called with output messages, so that other clients can use this (e.g. nbconvert) Default is to redisplay, including detection of IPython kernel for rich outputs.

minrk force-pushed the KernelClient.run branch from 2b37871 to 9687697 Compare August 11, 2016 14:52

minrk changed the title ~~add BlockingKernelClient.run~~ add BlockingKernelClient.execute_interactive Aug 11, 2016

add stdin_hook

10dbc76

so that input request handling can be customized

takluyver added this to the 5.0 milestone Aug 11, 2016

takluyver reviewed Aug 11, 2016
View reviewed changes

zmq timeouts are milliseconds

d511480

(annoying)

takluyver modified the milestones: 4.5, 5.0 Aug 11, 2016

review comments

01133c8

minrk mentioned this pull request Aug 11, 2016

[WIP] use new KernelClient.execute_interactive jupyter/nbconvert#360

Closed

minrk mentioned this pull request Sep 9, 2016

handle other outputs on pt eventloop jupyter/jupyter_console#105

Open

minrk merged commit 863ead4 into jupyter:master Sep 26, 2016

minrk deleted the KernelClient.run branch September 26, 2016 13:06

minrk modified the milestones: 4.5, 5.0 Feb 7, 2017

add BlockingKernelClient.execute_interactive #185

add BlockingKernelClient.execute_interactive #185

Conversation

minrk commented Aug 11, 2016 • edited Loading

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

dsblank commented Aug 11, 2016

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

Choose a reason for hiding this comment

minrk Aug 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsblank commented Aug 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsblank commented Aug 11, 2016

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

dsblank commented Aug 11, 2016

minrk commented Aug 11, 2016

takluyver commented Aug 11, 2016

minrk commented Aug 11, 2016 • edited Loading

takluyver commented Aug 11, 2016

minrk commented Aug 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takluyver commented Aug 11, 2016

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

takluyver commented Aug 11, 2016

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

minrk commented Aug 11, 2016

ivan-krukov commented Sep 7, 2016 • edited Loading

minrk commented Sep 8, 2016

ivan-krukov commented Sep 8, 2016

minrk commented Sep 9, 2016

minrk commented Sep 12, 2016

minrk commented Aug 11, 2016 •

edited

Loading

minrk Aug 11, 2016 •

edited

Loading

minrk commented Aug 11, 2016 •

edited

Loading

ivan-krukov commented Sep 7, 2016 •

edited

Loading