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

Hook into IPython's display #1

Open
twavv opened this issue Jul 26, 2019 · 7 comments
Open

Hook into IPython's display #1

twavv opened this issue Jul 26, 2019 · 7 comments

Comments

@twavv
Copy link
Collaborator

twavv commented Jul 26, 2019

We need to forward IPython.display.display to actually send things to the IOPub channel (something in the process is not working right now).

@twavv
Copy link
Collaborator Author

twavv commented Jul 29, 2019

I did this, but I'd still like to be able to use IPython's stuff for sending the final execute_result message that way we don't have to special-case the widget MIME (and try to show it for every PyObject that is shown).

@stevengj
Copy link
Member

Why is IPython sending an execute_result? IJulia should be the only thing receiving an execute_request and responding with an execute_result.

IPython's display might be sending display_data.

message that way we don't have to special-case the widget MIME

Similar to how PyCall does it, we just need to define something like:

Base.showable(::MIME"application/vnd.jupyter.widget-view+json", o::PyObject) =
    !ispynull(o) && PyCall.hasproperty(o, "_model_id")

@twavv
Copy link
Collaborator Author

twavv commented Jul 29, 2019

It's not, but hooking into that allows us to not special case widgets. Any object that defines a custom mime type in python should™ just work (this is what I was trying to make possible in my PR to IJulia).

So it's not that ipython is sending the messages, but rather, I'd like for IJulia/PyInteract to hook into the way objects are converted to MIME bundles so that we can send that bundle as part of IJulia's execute request.

@stevengj
Copy link
Member

I'd like for IJulia/PyInteract to hook into the way objects are converted to MIME bundles

It used to be that this was just via _repr_html_ methods etcetera, which is what PyObject supports now.

Nowadays IPython allows objects to define _repr_mimebundle_ and _ipython_display_ methods, so I guess we should support those?

@twavv
Copy link
Collaborator Author

twavv commented Jul 29, 2019

I'm partial to just trying to re-use as much of the existing machinery as possible. For reference, this is what the actual code looks like (assuming that my IJulia PR is merged as is):

function IJulia.display_payload(object::PyObject)
    # The payload returned is of form [data, metadata].
    payload = IJuliaInteractiveShell.instance().display_formatter.format(object)
    if payload === nothing || isempty(payload[1])
        return nothing
    end

    # Make sure we keep the `PyObject <...>` printing
    payload[1]["text/plain"] = sprint(show, object)

    return (data=payload[1], metadata=payload[2])
end

No special handling, no dealing with dunder methods. I think the more we stick to this kind of approach, the less weird edge cases we're going to run into.

@stevengj
Copy link
Member

The problem with calling IPython.display_formatter is that it creates a dependency on IPython.

A Python package Foo could easily have defined _repr_html_ or _repr_mimebundle_ without depending on IPython directly, and I don't think PyCall should force IPython to be installed in order for Foo's types to display correctly.

@twavv
Copy link
Collaborator Author

twavv commented Jul 29, 2019

We're already depending on IPython though (via ipykernel).

I think all of this stuff can be completely orthogonal to PyCall and the way it displays objects. The display_payload thing would only be for IJulia (which I think is desirable).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants