allow formatters to specify metadata #3190

minrk · 2013-04-16T20:36:06Z

A formatter can now return (data, metadata), in addition to just data.

A display_data message may now have a key in the content's existing metadata field for each format type.
These keys are not necessarily defined.

The main initial use of this code is to allow images to specify display size,
which should be the missing piece before implementing 2x figures (#2234).

Pinging @piti118, who plans to do that implementation on top of this.

ellisonbg · 2013-04-25T22:25:23Z

IPython/core/display.py

+        """shortcut for returning metadata with shape information, if defined"""
+        md = {}
+        if self.width:
+            md['width'] = self.width


What are the units of width and height here?

Integer pixels make the most sense, but technically anything HTML allows will work ("2em", etc.)

ellisonbg · 2013-04-25T22:44:51Z

Previously the display publisher had the ability to publish a metadata dict, but we never used it. As I understand this PR, this does a couple of things:

It starts to use that metadata dict.
It standardizes the top-level keys of the metadata dict (mime types).
It allows formatters to pass in data to that metadata dict by returning a (format, md) tuple.

If this is correct we could use this to allow JS code to declare the name of a browser side handler in the metadata?

minrk · 2013-04-25T22:57:15Z

That seems correct. The metadata dict is currently a black box. This simply defines that mime-type keys are themselves black boxes that will be passed to the append_mimetype handlers. So you can still add any further arbitrary interpretations of the metadata dict.

ellisonbg · 2013-04-26T04:09:13Z

Let's update the docstrings of the various classes/methods this PR touches to reflect that many return values are now a tuple of data, metadata, etc. Things like DisplayFormatter.format and friends.

ellisonbg · 2013-04-26T04:10:38Z

DisplayPublisher.publish docstring should describe how the metadata dict now has top level keys that are the format types.

ellisonbg · 2013-04-26T04:11:04Z

Same with docstring of publish_display_data.

ellisonbg · 2013-04-26T04:14:58Z

I am wondering if we should change how the metadata arg to functions like publish_html and friends should be interpreted. Right now, that metadata arg is just passed through to the underlying publish function. Another option would be to assume that the metadata passed is for that particular format type and automatically add it to the global metadata dict with the right top-level mime-type key. I think that makes more sense.

ellisonbg · 2013-04-26T04:18:20Z

The docstring and code of DisplayHook.compute_format_data is confusing. It looks and sounds like it will just return the raw data, but the method it calls returns the data, metadata tuple. The __call__ method that calls this one does the right thing, but it is still confusing.

ellisonbg · 2013-04-26T04:18:48Z

DisplayHook.write_format_data needs an updated docstring.

ellisonbg · 2013-04-26T04:24:15Z

Right now we use OutputArea.prototype._dblclick_to_reset_size to reset a resized image to its original size. Do we instead want to use the metadata provided sizes if they were passed?

minrk · 2013-04-26T04:37:03Z

I'm not fond of the publish_mime methods - they don't actually do what they describe, since they also publish plaintext for everything.

minrk · 2013-04-26T04:37:49Z

docstrings updated

minrk · 2013-04-26T04:40:56Z

Right now we use OutputArea.prototype._dblclick_to_reset_size to reset a resized image to its original size. Do we instead want to use the metadata provided sizes if they were passed?

We could, though the way we have it now will work, since it just uses the measured initial size, which will always be the specified size given with metadata, so it would be a bit redundant.

ellisonbg · 2013-04-26T04:43:07Z

Are you sure? From looking at the code, I think the publish_foo methods only publish their own mine type.

minrk · 2013-04-26T04:47:13Z

Oh, that's even worse - I was thinking of display, which always includes plaintext. publish doesn't for some reason. Why do the publish_foo methods exist?

ellisonbg · 2013-04-26T04:51:59Z

Originally, you couldn't publish metadata by calling display for using the
real formatters, so if you wanted to publish with metadata you had to use
these publish_foo methods. But now that we can publish data with
metadata, I think we don't need these. But we should add a metadata arg to
the main display classes (Image, HTML, Javascript, etc.), to make it easy
to publish data with metadata. Let's add that to this PR as it is a simple
change that will make the API more uniform. Then, I am fine removing the
publish methods if you want as well. Simplify, simplify, simplify...

On Thu, Apr 25, 2013 at 9:47 PM, Min RK notifications@github.com wrote:

Oh, that's even worse - I was thinking of display, which always
includes plaintext. publish doesn't for some reason. Why do the publish_foo
methods exist?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3190#issuecomment-17055508
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

ellisonbg · 2013-04-26T04:52:46Z

IOW, display and the top level display objects should be our official
public display protocol API, not these low-level publish ones...

On Thu, Apr 25, 2013 at 9:51 PM, Brian Granger ellisonbg@gmail.com wrote:

Originally, you couldn't publish metadata by calling display for using the
real formatters, so if you wanted to publish with metadata you had to use
these publish_foo methods. But now that we can publish data with
metadata, I think we don't need these. But we should add a metadata arg to
the main display classes (Image, HTML, Javascript, etc.), to make it easy
to publish data with metadata. Let's add that to this PR as it is a simple
change that will make the API more uniform. Then, I am fine removing the
publish methods if you want as well. Simplify, simplify, simplify...

On Thu, Apr 25, 2013 at 9:47 PM, Min RK notifications@github.com wrote:

Oh, that's even worse - I was thinking of display, which always
includes plaintext. publish doesn't for some reason. Why do the publish_foo
methods exist?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3190#issuecomment-17055508
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

minrk · 2013-04-26T04:56:10Z

we want a metadata dict kwarg on all display types? What should happen if someone specifies width/height as kwargs and via metadata? Or do we want to pass any **kwargs through as metadata?

ellisonbg · 2013-04-26T04:59:44Z

Because we need to tightly monitor all metadata attributes, lets set the
metadata via specific keywords args like width/height. That way we can do
a better job checking their formats. For example, if we introduce a
handler metadata attr for the JSON mime-type, users would pass that in as a
handler kwargs to the display object.

On Thu, Apr 25, 2013 at 9:56 PM, Min RK notifications@github.com wrote:

we want a metadata dict kwarg on all display types? What should happen if
someone specifies width/height as kwargs and via metadata? Or do we
want to pass any **kwargs through as metadata?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3190#issuecomment-17055696
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

ellisonbg · 2013-04-26T05:06:14Z

Important question: is this metadata saved in the notebook format? Seems like it should be otherwise when a notebook is reloaded, the metadata will be lost. If it is part of the notebook format, is that not a notebook format change?

minrk · 2013-04-26T05:08:18Z

yes, the metadata is saved in the notebook

formatters can now return either data or (data, metadata) the resulting published dict has both content.data keyed by mime-types,a and content.metadata keyed by mime-types. metadata keys are not necessarily defined, and are generally undefined when they would have no content. This fits fully within the existing message spec.

also consolidate a bunch of duplicate code

adds display(raw=True), which maps directly to publish_display_data on each arg.

ellisonbg · 2013-04-29T21:55:48Z

So I think this code is ready to merge. The only thing we talked about was updating the message spec/nbformat documentation with information about the metadata. Do you want to do that in a separate PR, or include it here?

minrk · 2013-04-29T21:56:51Z

Ah, I updated the IPEP, but forgot to update the actual docs. I'll do that now. I will also add an example or two to the example notebooks.

ellisonbg · 2013-04-29T21:57:15Z

OK great!

minrk · 2013-04-29T22:09:02Z

docs should be updated

ellisonbg · 2013-04-29T22:12:59Z

Merging, great work!

allow formatters to specify metadata

ellisonbg reviewed Apr 25, 2013
View reviewed changes

minrk added 8 commits April 29, 2013 11:27

publish image shape as metadata

a0cdd9f

support display_pub metadata in js frontend

a6cc6c4

format-related docstrings!

7391e81

remove publish_foo functions

853e909

add metadata to display functions

c6b25e5

also consolidate a bunch of duplicate code

do't define redundant _publish in core.display

5466d31

display_mime(objs, metadata) puts metadata into mime type key

603cf7e

adds display(raw=True), which maps directly to publish_display_data on each arg.

minrk added 2 commits April 29, 2013 15:06

discuss display metadata in messaging doc

8947eec

add metadata to nbformat Python API

b318111

ellisonbg added a commit that referenced this pull request Apr 29, 2013

Merge pull request #3190 from minrk/image_size

cf5a12a

allow formatters to specify metadata

ellisonbg merged commit cf5a12a into ipython:master Apr 29, 2013

stevengj mentioned this pull request Aug 5, 2013

RFC: add multimedia I/O mechanism (display, mm_write, and friends); fixes #3817 JuliaLang/julia#3932

Merged

minrk deleted the image_size branch March 31, 2014 23:36

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request ipython#3190 from minrk/image_size

ac73ee6

allow formatters to specify metadata

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow formatters to specify metadata #3190

allow formatters to specify metadata #3190

minrk commented Apr 16, 2013

ellisonbg Apr 25, 2013

minrk Apr 25, 2013

ellisonbg commented Apr 25, 2013

minrk commented Apr 25, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

minrk commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 29, 2013

minrk commented Apr 29, 2013

ellisonbg commented Apr 29, 2013

minrk commented Apr 29, 2013

ellisonbg commented Apr 29, 2013

allow formatters to specify metadata #3190

allow formatters to specify metadata #3190

Conversation

minrk commented Apr 16, 2013

ellisonbg Apr 25, 2013

Choose a reason for hiding this comment

minrk Apr 25, 2013

Choose a reason for hiding this comment

ellisonbg commented Apr 25, 2013

minrk commented Apr 25, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

minrk commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

ellisonbg commented Apr 26, 2013

minrk commented Apr 26, 2013

ellisonbg commented Apr 29, 2013

minrk commented Apr 29, 2013

ellisonbg commented Apr 29, 2013

minrk commented Apr 29, 2013

ellisonbg commented Apr 29, 2013