-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support display_id in execute preprocessor #563
Conversation
You've got a merge conflict on this - looks like they were old commits but you just got round to making a PR? |
nbconvert/preprocessors/execute.py
Outdated
@@ -154,6 +153,8 @@ class ExecutePreprocessor(Preprocessor): | |||
config=True, | |||
help='The kernel manager class to use.' | |||
) | |||
|
|||
_display_id_map = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a comment here describing the structure of the data stored in this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Added.
needed for updating current cell outputs
by passing cell_index to run_cell
Rebased. Not sure why Travis doesn't feel like running... |
Travis has decided to run now, and there's a failure on Python 2, when it tries to start a Python 3 kernel for the new notebook. Have we done something to get round that for the other notebooks? |
this is icky, but seems to be what neighbor notebooks do
I looked around and only saw that the test notebooks lack kernel metadata, so launch implicitly with the default kernel. Seems like we should have a better mechanism for that, but I followed the rest in this PR. |
This looks good to me, but I'll give @mpacer a chance to look at it as well. |
I had looked at this after the initial commits but didn't say anything because I didn't know the context of this. I don't see anything to take issue with at second glance, but what is this accomplishing in the larger picture (which will help me think about the code in greater depth)? |
Recent versions of IPython and notebook added a mechanism for updating (replacing, really) an output that has already been displayed. When you display an output, you can give it an ID (or ask IPython to assign it a random ID and return that to you). That ID is sent in the metadata of the output message. If another output message comes later with the same display ID in its metadata, the frontend will replace the previous output with that ID, rather than adding a new one. This can be used to make a progress bar, for instance. (I've deliberately handwaved the details a bit because I don't remember them, but hopefully they're documented in the message spec.) This PR adds support for this when nbconvert is executing a notebook. |
kk. LGTM. |
Added in protocol v5.1, supported in notebook 5.0.
display_id allows updating existing outputs, so keep track of mapping of display_id to outputs.