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 ability to hide outputs in Notebooks #109104

Closed
DonJayamanne opened this issue Oct 21, 2020 · 8 comments
Closed

Add ability to hide outputs in Notebooks #109104

DonJayamanne opened this issue Oct 21, 2020 · 8 comments
Assignees
Labels
feature-request Request for new features or functionality notebook-output verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link
Contributor

Request for ability to hide output from notebooks.

Currently in the Python implementation of Notebooks, we have output that are hidden.

  • Today these are neither displayed in our old Python Notebooks nor in Jupyter
  • As far as I can tell, most of these outputs end up as Rich Displays, containing an object with a single key. However no real data for display purposes.
  • The outputs are not displayed, however they are persisted on disck (i.e. they are part of the ipynb content).

Adding an issue for what was discussed (forgot to add this issue last week).
@rebornix /cc

@rebornix rebornix added feature-request Request for new features or functionality notebook labels Nov 2, 2020
@rebornix rebornix added this to the Backlog milestone Nov 2, 2020
@rebornix rebornix modified the milestones: Backlog, November 2020 Nov 17, 2020
@rebornix rebornix added the verification-needed Verification of issue is requested label Dec 1, 2020
@RMacfarlane RMacfarlane added the verification-steps-needed Steps to verify are needed for verification label Dec 3, 2020
@RMacfarlane
Copy link
Contributor

@rebornix What's a good way to verify this change?

@DonJayamanne
Copy link
Contributor Author

@DonJayamanne to provide logic for hiding empty ipywidget output.

@DonJayamanne
Copy link
Contributor Author

@rebornix Unfortunately the logic for rendering a widget is in the widget manager.
Here's the sample code:

import nglview
view = nglview.show_pdbid("3pqr")
view

When we run this we get two outputs with almost identical data, but with different model_ids, see below:

   "outputs": [
    {
     "data": {
      "application/vnd.jupyter.widget-view+json": {
       "model_id": "d2fc10942d3e4a0d93f055c7f85bdf5d",
       "version_major": 2,
       "version_minor": 0
      },
      "text/plain": ""
     },
     "metadata": {
      "transient": {}
     },
     "output_type": "display_data"
    },
    {
     "data": {
      "application/vnd.jupyter.widget-view+json": {
       "model_id": "bfc1da481100435990053406e1c8348f",
       "version_major": 2,
       "version_minor": 0
      },
      "text/plain": "NGLWidget()"
     },
     "metadata": {
      "transient": {}
     },
     "output_type": "display_data"
    }
   ],

Unfortunately, only the widget manager knows which one needs to be hidden.
Basically we query the widget manager to see if it has any views to be rendered for the provided model_id. If there isn't any view associated with the model_id, then we don't render it.

With the renderer architecture, the only way to determine whether a cell needs to be rendered or not can only be provided by the renderer.

Finally, the renderer doesn't render the output immediately.
Just because the renderer was asked to render an output for model_id = xyz doesn't mean it will render the output immediately. This is all asynchronous.

@rebornix
Copy link
Member

rebornix commented Dec 7, 2020

@DonJayamanne I guess we can have a workaround for this by doing:

  • When we see a two mimetype output (pretty common in Jupyter), one is custom mimtype, which will be rendered in the webview, another one is text/plain, which will be rendered by VS Code itself. If the custom mimetype rendered in the webview has height of zero and the text/plain mimetype is empty string, we then not preserve any whitespace in the output container and will also hide the mimetype switcher.
    • We can even limit this behavior to application/vnd.jupyter.widget-view+json/text/plain mimetype bundle only
  • As long as the widget has some height, we will render them just like what we do now

@DonJayamanne
Copy link
Contributor Author

Sure, but my concern is if there are two legitimate outputs then VSCode might end up removing the padding.
Because VS Code code will probably run before the renderer code (renderer code is async & needs to wait for the data to come through from kernel into the widget manager).
Hence possible that VS Code will assume the output is empty (0 height) & remove the padding & after that the widget manager renders the output.

@rebornix
Copy link
Member

rebornix commented Dec 8, 2020

Hence possible that VS Code will assume the output is empty (0 height) & remove the padding & after that the widget manager renders the output.

When this happens, we will automatically bring back the padding, it has a listener to the content height change. Does this fix the problem?

@amunger
Copy link
Contributor

amunger commented Nov 19, 2024

I think this was fixed along with #214886, though that never got verified.
Empty outputs should be combined into a single output

@amunger amunger closed this as completed Nov 19, 2024
@amunger amunger added verification-needed Verification of issue is requested author-verification-requested Issues potentially verifiable by issue author labels Nov 19, 2024
@amunger amunger modified the milestones: Backlog, November 2024 Nov 19, 2024
@alexr00 alexr00 added the ~verification-steps-needed Steps to verify are needed (with bot comment) label Dec 3, 2024
Copy link

Friendly ping! Looks like this issue requires some further steps to be verified. Please provide us with the steps necessary to verify this issue.

@vs-code-engineering vs-code-engineering bot added verification-steps-needed Steps to verify are needed for verification and removed ~verification-steps-needed Steps to verify are needed (with bot comment) labels Dec 3, 2024
@rzhao271 rzhao271 added verified Verification succeeded and removed verification-steps-needed Steps to verify are needed for verification author-verification-requested Issues potentially verifiable by issue author labels Dec 3, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality notebook-output verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants
@rebornix @DonJayamanne @amunger @RMacfarlane @rzhao271 @alexr00 and others