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

Render an empty widget if we don’t have a renderer for the mimebundle types. #6559

Merged
merged 1 commit into from Jun 13, 2019

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jun 12, 2019

References

Fixes #6216

Code changes

This does render a blank non-classed div for outputs corresponding to things we can’t render, since we need the list of widgets to correspond to the output messages. So it's not exactly like the classic notebook (which doesn't render anything for things it doesn't understand), but from a user perspective perhaps this is good enough.

I suppose another way to do this is instead of all of the special-casing with adding class names, just hide the widget, which should do a display:none. However, we should check to make sure that the widget children iteration counts hidden widgets.

User-facing changes

There no longer is an error if we try to display a display_data message that has no recognizable mimetypes.

Backwards-incompatible changes

… types.

Fixes jupyterlab#6216

This does render a div for outputs corresponding to things we can’t render, since we need the list of widgets to correspond to the output messages.
@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Jun 12, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@jasongrout jasongrout added this to the 1.0 milestone Jun 12, 2019
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jun 13, 2019

Looks good to me, thanks @jasongrout!

@ian-r-rose ian-r-rose merged commit 8e2ed72 into jupyterlab:master Jun 13, 2019
9 checks passed
@vidartf
Copy link
Member

@vidartf vidartf commented Jun 13, 2019

Can we at least have a console log/warning? It seems bad to fail to render a cell output, with no hints to the user about what can have gone wrong.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jun 13, 2019

A warning to the console sounds reasonable to me.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 13, 2019

Note that in this case (with papermill workflow), having an unrecognized mime type was supposed to happen, so it could generate quite a few errors with a console log/warning. I think generally things will have at least a text/plain mimetype.

@vidartf
Copy link
Member

@vidartf vidartf commented Jun 13, 2019

Sure, I just won't respond to any ipywidgets questions that are about why they are only getting the repr string ;)

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 13, 2019

Those cases aren't affected by this, because they do have a text representation, so the code is happy before and after this change just the same.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 13, 2019

(this codepath is only exercised if there is no mimetype understood - that should be extremely rare in most cases. Except when using papermill, apparently.)

@vidartf
Copy link
Member

@vidartf vidartf commented Jun 13, 2019

@jasongrout Or if the mimetype is unsafe.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 13, 2019

@jasongrout Or if the mimetype is unsafe.

Right, no mimetype understood includes the current safety level allowed. But again, the vast majority of mimebundles should have a text/plain representation (I thought all would), so this codepath isn't exercised hardly ever.

@lock
Copy link

@lock lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related discussion.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants