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

Decide how to handle displayhook being triggered multiple times #350

Closed
ellisonbg opened this issue Apr 10, 2011 · 8 comments
Closed

Decide how to handle displayhook being triggered multiple times #350

ellisonbg opened this issue Apr 10, 2011 · 8 comments
Assignees
Milestone

Comments

@ellisonbg
Copy link
Member

In IPython master, displayhook can currently be triggered more than one time per input. Previously we had talked about only allowing it to trigger once per input. We need to decide on the desired behavior and implement it. R. Kern brought up the good point that we don't need to handle this in the block splitter code, but could handle it in our custom displayhook code by allow the trigger to happen multiple times, but only showing the last result.

@ghost ghost assigned fperez Apr 10, 2011
@takluyver
Copy link
Member

I think handling it in displayhook would lead to unexpected behaviour. Specifically, if I do for a in range(5): a, I would expect to see either five outputs or none.

Two possible ways to do this in AST side: we can run the last node interactively only if it's an expression, or we can run the last node which is an expression interactively, then switch back to exec mode for the remaining nodes. I think I would find the former behaviour less surprising (that is, only the last node can produce output).

@takluyver
Copy link
Member

If people want to test it, here's a branch doing what I think is the most intuitive behaviour:

https://github.com/takluyver/ipython/tree/single-output

@ellisonbg
Copy link
Member Author

Fernando and I tested this out and we really like the behavior. We
are +1 on merging this, but wanted to check with you first about any
issues related to this and how the history is stored. Our
understanding is that the sqlite history is setup to only store 0 or 1
Out prompts, so the behavior of this branch is completely consistent
with the current state of the sqlite history. Is this accurate? Do
you want to make any other changes or the related history stuff before
we merge? Pull request?

On Sun, Apr 10, 2011 at 2:27 PM, takluyver
reply@reply.github.com
wrote:

If people want to test it, here's a branch doing what I think is the most intuitive behaviour:

https://github.com/takluyver/ipython/tree/single-output

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

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

@takluyver
Copy link
Member

Actually, in its current state, the history can store any number of outputs, by sticking them into a JSON list. If we like this behaviour, I'll change that back so that it expects 0 or 1 output, and just stores it individually.

@ellisonbg
Copy link
Member Author

Yes, Fernando and I talked extensively about this last night and think
this is the way to go. I do think it makes sense to have the JSON
structure enforce the 0 or 1 output constraint. We will be enforcing
this in other places like the html notebook format. Let me know when
you have a pull request and I can review+merge this.

On Mon, Apr 11, 2011 at 2:43 AM, takluyver
reply@reply.github.com
wrote:

Actually, in its current state, the history can store any number of outputs, by sticking them into a JSON list. If we like this behaviour, I'll change that back so that it expects 0 or 1 output, and just stores it individually.

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

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

@takluyver
Copy link
Member

I'll probably lose the JSON altogether, and just store the text/plain repr
in the database.

@ellisonbg
Copy link
Member Author

I think this sounds good.

On Mon, Apr 11, 2011 at 10:39 AM, takluyver
reply@reply.github.com
wrote:

I'll probably lose the JSON altogether, and just store the text/plain repr
in the database.

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

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

@takluyver
Copy link
Member

Branch merged at bb38481, so I'm closing this issue.

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

No branches or pull requests

3 participants