-
Notifications
You must be signed in to change notification settings - Fork 280
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
Rich display is broken on recent version of ipython #4054
Comments
I honestly did not expect that we'd get this trouble using all ipython stuff! I thought we were doing the right thing. 😊 Anyway, for reference, here's the implementation: def _ipython_display_(self):
import ipywidgets
from IPython.display import display
fnames = []
children = []
for ftype in sorted(self.field_types):
fnc = getattr(self, ftype)
children.append(ipywidgets.Output())
with children[-1]:
display(fnc)
fnames.append(ftype)
tabs = ipywidgets.Tab(children=children)
for i, n in enumerate(fnames):
tabs.set_title(i, n)
display(tabs) and each of the sub-displays is: def _ipython_display_(self):
import ipywidgets
from IPython.display import Markdown, display
names = dir(self)
names.sort()
def change_field(_ftype, _box, _var_window):
def _change_field(event):
fobj = getattr(_ftype, event["new"])
_box.clear_output()
with _box:
display(
Markdown(
data="```python\n"
+ textwrap.dedent(fobj.get_source())
+ "\n```"
)
)
values = inspect.getclosurevars(fobj._function).nonlocals
_var_window.value = _fill_values(values)
return _change_field
flist = ipywidgets.Select(options=names, layout=ipywidgets.Layout(height="95%"))
source = ipywidgets.Output(layout=ipywidgets.Layout(width="100%", height="9em"))
var_window = ipywidgets.HTML(value="Empty")
var_box = ipywidgets.Box(
layout=ipywidgets.Layout(width="100%", height="100%", overflow_y="scroll")
)
var_box.children = [var_window]
ftype_tabs = ipywidgets.Tab(
children=[source, var_box],
layout=ipywidgets.Layout(flex="2 1 auto", width="auto", height="95%"),
)
ftype_tabs.set_title(0, "Source")
ftype_tabs.set_title(1, "Variables")
flist.observe(change_field(self, source, var_window), "value")
display(
ipywidgets.HBox(
[flist, ftype_tabs], layout=ipywidgets.Layout(height="14em")
)
) I wrote this under the impression that we'd only be calling the rich displays when we were in something that could handle them -- I think we'll have to update them for the latest version. (I really was trying to do the right thing in building it this way!) I hope we don't have to have a version conditional, or break older versions with new. |
You know, it kind of looks to me like we might be having issues with the |
After a bit of investigation, this is the new behaviour in IPython 8+. I see multiple solutions Info
In a perfect world, we would replace our calls to Links: The ipywidget solutionWith the upcoming ipywidget 8 (not released yet), widgets will have a import ipywidgets as widgets
class Test:
def _repr_mimebundle_(self, include=None, exclude=None):
t = widgets.Text("HTML display")
mimebundle = t._repr_mimebundle_()
mimebundle["text/plain"] = "<plain>"
return mimebundle
Test() # this will display "HTML display" in notebooks, "<plain>" in IPython (any version!) and Test.__repr__() otherwise. See jupyter-widgets/ipywidgets#2950. Check IPython & interactivityWe can also check explicitly that we are in a notebook environment that can display “rich” info: import ipywidgets as widgets
def is_notebook() -> bool:
# Adapted from https://stackoverflow.com/a/39662359
try:
shell = get_ipython().__class__.__name__
if shell in ("ZMQInteractiveShell", "google.colab._shell"):
return True
else:
return False
except NameError:
return False # Probably standard Python interpreter
class Test:
def _ipython_display_(self):
from IPython.display import display
if is_notebook():
display(widgets.Text("HTML display"))
else:
display("<plain>")
Test() # this will display "HTML display" in notebooks, "<plain>" in IPython 8+ and Test.__repr__() otherwise. Wrap widget in “displayer”At the moment the "text" display of ipywidgets is the representation of the widget. We could very well wrap all our rich displays into a tailored class that uses a plain value that's more relevant. from dataclasses import dataclass
import ipywidgets as widgets
@dataclass
class WidgetWrapper:
widget: widgets.Widget
plaintext_repr: str
def _ipython_display_(self, **kwargs):
# Adapted from ipywidgets.widgets.widget.Widget._ipython_display_
plaintext = self.plaintext_repr
if len(plaintext) > 110:
plaintext = plaintext[:110] + '…'
data = {
'text/plain': plaintext,
}
if self.widget._view_name is not None:
# The 'application/vnd.jupyter.widget-view+json' mimetype has not been registered yet.
# See the registration process and naming convention at
# http://tools.ietf.org/html/rfc6838
# and the currently registered mimetypes at
# http://www.iana.org/assignments/media-types/media-types.xhtml.
data['application/vnd.jupyter.widget-view+json'] = {
'version_major': 2,
'version_minor': 0,
'model_id': self.widget._model_id
}
display(data, raw=True)
if self.widget._view_name is not None:
self.widget._handle_displayed(**kwargs)
def __repr__(self):
return self.plaintext_repr
w = WidgetWrapper(widgets.Text("HTML display"), "<plain>")
w # this will display "HTML display" in notebooks, "<plain>" otherwise. |
Any chance you know why this change was made? |
My understanding was that some modern terminal emulators (like iterm2) are actually able to display images and stuff, so moving into the future, we could display rich information in terminals. |
btw, when ipywidget isn't installed, calling (within IPython) ds.fields crashes with
I suppose it shouldn't try to use fancy rich repr in this case, right ? edit: reported as a separate issue (#4154) |
@cphyc ipywidget 8 is now available. I think it would be reasonable to require it and implement |
@cphyc how comfortable do you feel about doing this and do you think you can manage the time for it soon-ish ? I'm thinking this would fit very well (thematically) with other fixes already on their way to yt 4.1.1 |
So, a quick question -- if we're comfortable implementing this, do you think that we could start implementing repr bundles that utilize widgyts directly, without the need for monkeypatching? Back in the long-long ago, it was possible to define entry points for different packages and to use that to provide plugin functionality. i.e., if a particular package was installed, it could be "checked" for without actually importing it. Can we still do that? If so, it'd be great to just start doing it when we do this refactoring. |
Importlib (part of the standard library) has APIs to check if a package is installed without importing it. We use it in ˋconftest.py` to filter warnings depending on which optional dependencies are available. |
Bug report
Bug summary
On IPython ≥8, yt fields are displayed as if we were in a rich display interface (but we're not!). See screenshot.
Code for reproduction
In an IPython shell:
Actual outcome
Expected outcome
Version Information
The text was updated successfully, but these errors were encountered: