Skip to content
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

(1) Enable IPython.notebook.kernel.execute to publish display_* even it is not called with a code cell and (2) remove empty html element when execute "display_*" #1441

Closed

Conversation

cschin
Copy link

@cschin cschin commented Feb 27, 2012

(1) If "IPython.notebook.kernel.execute" is called without a cell attached, the "display_javascript" or "display_html" is currently ignored by Notebook.prototype.handle_shell_reply() since there is no cell associated with the python code execution request. The patch allows the message type "display_data" to be accepted by Notebook.prototype.handle_shell_reply() even there is no "cell" attached with the initial IPython.notebook.kernel.execute() request. (For a better design, we might consider to assign such call with special cell id, or make a new type of message.)

(2) Removing html insertion for "display_*" calls. If we put "display_javascript" in a loop, the current noteboot will insert empty html elements and the output cell might become longer (and empty) than necessary. It looks cleaner not generating those empty element.

The patches can allow to execute python through html element, e.g., a button created with javascript in an ipython html notebook. (one example: https://github.com/cschin/IPython-Notebook---d3.js-mashup/blob/master/GDP_CO2_Example.ipynb )

…Python.notebook.kernel.execute(code). Also clean up unnecessary output div while using publish_javascript.
@minrk
Copy link
Member

minrk commented Feb 27, 2012

We must be careful, because the exclusion of code the notebook didn't execute is actually deliberate and important behavior. We do not want output caused by other frontends (e.g. qtconsole) to appear in the notebook.

@ellisonbg
Copy link
Member

You have correctly identified a problem with our Javascript code, but
I don't think this solution will work. We need to make sure that
every call to execute is correctly hooked up with code that can handle
the output appropriately. That requires a good bit of refactoring to
Kernel, Notebook and Cell. We have already started to design the
replacement API for this but it is going to take a bit of work to pull
it off. Because these APIs are going to be public, we really need to
get them right. We are getting ready to release 0.13 and this work
will need to wait until after that. With any work on the notebook, we
highly recommend contacting us on list first because of the fast pace
things are moving at. We appreciate your contribution though and
definitely want to enable this type of thing. We will let the list
know when we are ready to start on this work.

On Sun, Feb 26, 2012 at 9:49 PM, Jason Chin
reply@reply.github.com
wrote:

(1) If "IPython.notebook.kernel.execute" is called without a cell attached, the "display_javascript" or "display_html" is currently ignored by Notebook.prototype.handle_shell_reply() since there is no cell associated with the python code execution request.  The patch allows the message type "display_data" to be accepted by Notebook.prototype.handle_shell_reply() even there is no "cell" attached with the initial IPython.notebook.kernel.execute() request.  (For a better design, we might consider to assign such call with special cell id, or make a new type of message.)

(2) Removing html insertion for "display_*" calls. If we put "display_javascript" in a loop, the current noteboot will insert empty html elements and the output cell might become longer (and empty) than necessary. It looks cleaner not generating those empty element.

The patches can allow to execute python through html element, e.g., a button created with javascript in an ipython html notebook. (one example: https://github.com/cschin/IPython-Notebook---d3.js-mashup/blob/master/GDP_CO2_Example.ipynb )

You can merge this Pull Request by running:

 git pull https://github.com/cschin/ipython 3a34d3b

Or you can view, comment on it, or merge it online at:

 #1441

-- Commit Summary --

  • Allowing to publish javascript within the code being passed through IPython.notebook.kernel.execute(code). Also clean up unnecessary output div while using publish_javascript.

-- File Changes --

M IPython/frontend/html/notebook/static/js/codecell.js (18)
M IPython/frontend/html/notebook/static/js/notebook.js (16)

-- Patch Links --

 https://github.com/ipython/ipython/pull/1441.patch
 https://github.com/ipython/ipython/pull/1441.diff


Reply to this email directly or view it on GitHub:
#1441

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg ellisonbg closed this Feb 27, 2012
@cschin
Copy link
Author

cschin commented Feb 27, 2012

Hi, sorry for not contacting you on the list before the PR. I am wondering where I will get some information on refactoring notebook. I agree with you totally. What I am looking for is the right solution too, but without knowing what you have in mind it might not easy for me to see the right path for the current architecture design.

@minrk
Copy link
Member

minrk commented Feb 27, 2012

No harm at all in having this conversation here as opposed to on the list. I'll let Brian describe where the architecture needs to go for the kernel to be used as a library, as opposed to being a wholly integrated app.

@cschin
Copy link
Author

cschin commented Feb 28, 2012

I will definitely like to see this work and will like to contribute. I am looking forward to know more about this so I can also help.

On Feb 27, 2012, at 1:27 PM, Min RK wrote:

No harm at all in having this conversation here as opposed to on the list. I'll let Brian describe where the architecture needs to go for the kernel to be used as a library, as opposed to being a wholly integrated app.


Reply to this email directly or view it on GitHub:
#1441 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants