Add width tags for jpeg and png when present #588
Conversation
The display(Image(..., retina=2)) command creates metadata about the image explaining the size that it should be displayed at in the notebook. This information, if present, can be carried into the notebook and converted to rst for use in documenters such as Sphinx. If not present, retina images appear in .rst (and from there to other formats such as HTML) as twice their intended size.
|
Thanks, this looks good to me. I assume you've tested this fix and it works? |
|
Yes, tested with both PNG and JPEG w/ and w/o retina=True flag. |
|
Great :-). I'd like @mpacer to have a quick look over this, but I'm happy for it to go in. |
|
Is It looks like we have a I'd slightly prefer it if we could test that this is adhered to so that this doesn't happen in the future. An aside: there's no way to update the image metadata in |
There was a problem hiding this comment.
This should also add an official test in nbconvert/exporters/tests/test_rst.py.
nbconvert/templates/rst.tpl
Outdated
| @@ -50,10 +50,16 @@ | |||
|
|
|||
| {% block data_png %} | |||
| .. image:: {{ output.metadata.filenames['image/png'] | urlencode }} | |||
| {%- if 'image/png' in output.metadata and output.metadata['image/png'].get('width', '') %} | |||
There was a problem hiding this comment.
It's cleaner to use the get_metadata filter. You can find an example of its use for an analogous purpose in the html template.
nbconvert/templates/rst.tpl
Outdated
| @@ -50,10 +50,16 @@ | |||
|
|
|||
| {% block data_png %} | |||
| .. image:: {{ output.metadata.filenames['image/png'] | urlencode }} | |||
| {%- if 'image/png' in output.metadata and output.metadata['image/png'].get('width', '') %} | |||
| :width: {{ output.metadata['image/png'].width}}px | |||
| {% endif -%} | |||
There was a problem hiding this comment.
You should also add height information if it is present.
nbconvert/templates/rst.tpl
Outdated
| {% endblock data_png %} | ||
|
|
||
| {% block data_jpg %} | ||
| .. image:: {{ output.metadata.filenames['image/jpeg'] | urlencode }} | ||
| {%- if 'image/jpeg' in output.metadata and output.metadata['image/jpeg'].get('width', '') %} |
There was a problem hiding this comment.
Same as above, use get_metadata and add height.
|
Thanks for "teaching me to fish" -- I've made these changes and added a test equivalent to the metadata test in test_html. I now feel like I understand the rst remplate system better so may be able to make some further improvements in the future. THANKS! @mpacer |
|
Happy to help — thank you for contributing! It's good as it stands, but if you want I'm confident that by tweaking the test to be closer to testing the behaviour you actually care about it can be even better. Happy to help with that too. But that's your call — if you want to call it "done" it's ready to be merged. |
|
@mscuthbert Did you want to keep working on this and learn a bit more or do you want me to merge it as is and make the changes myself? |
|
Also, I'm ok with you leaving the eclipse stuff there for now, but you should know that you could have just added them to .git/info/exclude. |
|
Hi @mpacer -- I'll try to get the changes in there soon. Thanks for the poke! |
|
@mscuthbert, is it ok if you make a separate PR to improve your test? I'm planning on releasing nbconvert today and I want to include the work you've done already. Does that seem reasonable? |
|
I'm just going to go ahead with that — it isn't too much overhead to make a new PR and then this feature will be available for everyone |
The display(Image(..., retina=True)) command creates metadata about the image explaining the size that it should be displayed at in the notebook. This information, if present, can be carried into the notebook and converted to rst for use in documenters such as Sphinx. If not present, retina images appear in .rst (and from there to other formats such as HTML) as twice their intended size.