Use _repr_mimebundle_ and require IPython 6.1 or later.

[jasongrout]

Fixes #1811

This has the problem (noted in the code) that the display callback now happens before the display message is sent. Before this, the display callback happened after the display message was sent.

CC @SylvainCorlay - what do you think about this? I'm thinking that this backwards compatibility difference would make this actually an 8.0 change, and may cause other problems.

[jasongrout]

Resetting to major release milestone until we figure out the display callback issue noted above.

[SylvainCorlay]

@jasongrout I think that it is very unlikely that this order has any consequences.

I can see two cases where this would be an issue:

• when someone overrides Jupyter's display hook to do more than sending the message, and they also rely on the display callbacks, and expect them to be called synchronously after the display hook is called.
• in cases where the display callbacks would send other messages to the frontend (such as instantiating other widgets) and rely on the front end processing messages in order.

Both situations seem very unlikely. I would be 👍 for merging this in the 7.x series.

[jasongrout]

in cases where the display callbacks would send other messages to the frontend (such as instantiating other widgets) and rely on the front end processing messages in order.

I thought that was one of the primary uses of the display callbacks.

[jasongrout]

I think a relevant question is: what are the on_displayed python-side hooks being used for? Does it make sense to get rid of the concept altogether? It's a bit of a weird concept when you have possibly multiple views of a widget.

CC also @maartenbreddels, who wanted the view count attribute, which is somewhat related.

[maartenbreddels]

I guess this should be raise NotImplemented?

[jasongrout]

No, NotImplemented is not an error class, so raising that will throw a TypeError.

[jasongrout]

(compare methods return NotImplemented, while subclasses with undefined methods raise NotImplementedError: https://docs.python.org/3.6/library/exceptions.html#NotImplementedError, https://docs.python.org/3.6/library/constants.html#NotImplemented)

[maartenbreddels]

Ah yes, but the docstring lies then. I cannot find which should be the right behaviour actually (return NotImplemented, or raise the error).

[flying-sheep]

or do what @SylvainCorlay said and just put the definition into an if statement.

[maartenbreddels]

I doubt the order matters, although you can add it to the ioloop to be executed later. But indeed, _view_count is more useful.

It does make it easier to use, and in order not to break too much, I'd say, let us break the order changing, but keep the event.

[SylvainCorlay]

in cases where the display callbacks would send other messages to the frontend (such as instantiating other widgets) and rely on the front end processing messages in order.

I thought that was one of the primary uses of the display callbacks.

Not specifically to create widgets though? Do you have an example?

[jasongrout]

Not specifically to create widgets though? Do you have an example?

I was searching for where on_displayed is used, and I couldn't find very many examples anywhere I looked on GitHub. It's used once in interact to run the function initially, for example.

Hence my question: where do people use this after all?

[SylvainCorlay]

@jasongrout the definition of _ipython_display_ could be behind a if statement.

    if ipython_version_info < (6,  1):
        def _ipython_display_(self, **kwargs):
            """Called when `IPython.display.display` is called on a widget.
            Note: only defined for IPython <6.1.
            """
            data = self._repr_mimebundle_(**kwargs)
            if data:
                display(data, raw=True)

[Madhu94]

Hi, any chance this version check can be made for IPython 5.4 and up? The _repr_mimebundle definition was backported to 5.x branch - ipython/ipython#10544

[flying-sheep]

that means the check would be the following?

if (5, 4) < ipython_version_info or (6, 0) <= ipython_version_info < (6, 1):
    def _ipython_display_(self, **kwargs):
         ...

[jasongrout]

Perhaps we just make a new major release of ipywidgets that cleans this up and updates our dependency on ipython. IPython 6.1 was released in May 2017 - it's been nearly two years, so I think it's okay to update the dependency.

Edit: this could also go along with dropping python 2 support in the next major release

[jasongrout]

Now this is rebased on #2654. Again, I looked in a number of packages, and no one seemed to be using the on_displayed python callback (either for something they needed, or even correctly propagating it to children), so I just removed it. ipyleaflet seemed to be the only one to propagate it to children, but no children were using it to do anything.

[SylvainCorlay]

ipyleaflet seemed to be the only one to propagate it to children, but no children were using it to do anything.

this is an artefact of an old version of ipywidgets when this was really used for all nested widgets.

[SylvainCorlay]

There seems to be commits unrelated to this in the PR (update to lumino etc).

[jasongrout]

There seems to be commits unrelated to this in the PR (update to lumino etc).

It is rebased on #2654, so it includes those commits (that should be merged first)

[jasongrout]

Never mind, I rebased instead on master so it is easier and less controversial to review/merge.

[jasongrout]

Unrelated failure in the new arm64 test run. I think if it fails a few more times, we should remove the arm64 test runs.

[jasongrout]

closed/reopened to kick the tests. I couldn't find a way to restart the tests using the travis ui.