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

Add transient_display_data message type and display such messages in console window #5699

Closed
wants to merge 4 commits into from

Conversation

BoPeng
Copy link
Contributor

@BoPeng BoPeng commented Nov 29, 2018

Re-implementation of PR #4879, reincarnation of proposal (#4550) on the display of transient_display_data (jupyter/jupyter_client#378).

This PR basically provides a way to send display_data to be displayed in console windows, not in notebooks. It

  1. Adds a new message type transient_display_data, which is derived directly from display_data with no additional field.
  2. Allow ForeignHandler of console to
  • display transient_display_data just like display_data when Show All Kernel Activity is turned on.
  • creates a new CodeCell with hidden input and output prompt and display only the transient_display_data.

image

What this means:

  1. Everything will be the same for kernels that do not support the transient_display_data message.
  2. For kernels that support this message type, it can send display_data for messages that are displayed in notebook (and associated console window if Show All Kernel Activity is enabled), and transient_display_data for messages that will only be displayed in the console windows.

As stated in the transient_display_data PR, this message type can be used to send arbitrary information that should not be stored in the main notebook permanently. Obvious examples include additional debug messages and progress information.

@afshin @jasongrout Let me know if this makes sense to you.

PS: I apologize for the use of an unofficial message type. However, I am in a chicken-egg situation where support from clients such as JLab is very important for the acceptance of the message type. I would love to see this PR accepted as an experimental feature that becomes official only after the message type is finalized.

@BoPeng
Copy link
Contributor Author

BoPeng commented Nov 30, 2018

@jasongrout had a look at the code this morning and commented that

  1. Using ForeignHandler to handle transient_display_data does not look clean, especially when transient messages are displayed when ForeignHandler is disabled.
  2. There can be cases when users would like to turn off transient_display_data. For example, when users do not want to see large amount of debug message, or if the users would like to "fix" a dataframe in the console window.

Jason suggested that transient_display_data should be handled separately from ForeignHandler, namely in a TransientHandler with its own toggle menu and preferably in an extension. The problems are that

  1. Currently ForeignHandler is tightly integrated in the console code. We will need to decouple console and ForeignHandler and make console extensible.
  2. A bigger problem is that transient_display_data messages are intermixed with other messages when Show All Kernel Activities are enabled. However, the _cells structure is private to ForeignHandler so a TransientHandler would not be able to insert transient messages into the cells without access to _cells. That is to say _cells and related creation functions need to go to console.

@BoPeng
Copy link
Contributor Author

BoPeng commented Nov 30, 2018

66df36e does the following:

  1. Rename ForeignHandler._enabled to _showAllActivity and add _showTransientMessage
  2. Add toggle menu Show Transient Message to control the display of transient messages independent of regular kernel messages.

Conceptually speaking, ForeignHandler handles all ioPub messages and there are two flags (menus) to control if it should display regular and/or transient messages from ioPub. This should address Jason's concerns except for the part that experimental code should better stay outside of the core.

@jasongrout
Copy link
Contributor

jasongrout commented Dec 1, 2018

@BoPeng - #5711 illustrates the pattern we talked about today, of having a separate third-party extension that is listening to messages and inserting cells. In particular, https://github.com/jupyterlab/jupyterlab/blob/1197fdf358a401ecfa73b618b3963e5be35d3532/packages/console-extension/src/foreign.ts and https://github.com/jupyterlab/jupyterlab/blob/1197fdf358a401ecfa73b618b3963e5be35d3532/packages/console/src/foreign.ts are the relevant files.

@BoPeng
Copy link
Contributor Author

BoPeng commented Dec 1, 2018

Still on the road, but thanks!

@jasongrout
Copy link
Contributor

@BoPeng and I discussed this today, and the conclusion was to (a) move the foreign message handling to a more self-contained plugin (done in #5711), and (b) move the transient message processing to a separate JupyterLab extension not in core so that it could iterate and experiment at a faster pace than core.

Thanks again @BoPeng for all of your work pushing these things forward, and being patient while we came up with a way to do this!

@jasongrout jasongrout closed this Dec 1, 2018
@jasongrout jasongrout added this to the Reference milestone Dec 1, 2018
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement pkg:console status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants