vega / vl-convert

Utilities for converting Vega-Lite specs from the command line and Python
BSD 3-Clause "New" or "Revised" License
100 stars 12 forks source link

Support ppi argument #85

Closed joelostblom closed 1 year ago

joelostblom commented 1 year ago

Vega-Lite 5.8 added support for a ppi argument to their CLI in https://github.com/vega/vega-lite/pull/8854. Being able to control the ppi of a figure is quite useful since that would give higher resolution without changing the displayed size (as scale/scale_factor does). Would it be possible to add a similar ppi setting to vl-convert so that it could be used both for saving figure and to display figures inline with the png renderer?

jonmmease commented 1 year ago

I've looked into this a bit, and I think it will depend on https://github.com/image-rs/image-png/pull/403

jonmmease commented 1 year ago

Based on https://github.com/vega/vl-convert/issues/84, I'm guessing that a motivation here is to get sharper images when using the Altair PNG renderer. For this case, we may be able to add something like the retina argument to IPython.display.Image (https://ipython.readthedocs.io/en/stable/api/generated/IPython.display.html#IPython.display.Image), where the altair PNG renderer would inspec the PNG image and set the display width to half the actual PNG size.

joelostblom commented 1 year ago

Based on https://github.com/vega/vl-convert/issues/84, I'm guessing that a motivation here is to get sharper images when using the Altair PNG renderer.

Exactly, that's what I am trying to achieve. I like the solution you mention about adding a flag for making images sharper if it is not easily possible to add a ppi argument until the issue you linked is closed

jonmmease commented 1 year ago

Try this:

from IPython.display import Image
import vl_convert as vlc

def display_png(chart, scale_factor=1, retina=True):
    if retina:
        scale_factor *= 2
    spec = chart.to_dict(format="vega")
    png = vlc.vega_to_png(spec, scale=scale_factor)
    return Image(png, retina=retina)

I think we could do something like this in the Altair PNG renderer, so you would call it like:

alt.renderers.enable("png", retina=True)

(and we could default to retina True of course)

This works in the notebook, but I haven't tried in an exported PDF. Do you want to give that a try?

joelostblom commented 1 year ago

Thanks for putting this together. The application I need this for is jupyter books generated with the --pdflatex flag. Unfortunately, it seems like the retina flag does not have an effect there and images with retina=True are not shrunk to half the size and width automatically. This is probably because there is no Jupyter frontend run during the Jupyter Book execution (I think), and this also seems to cause multiple other problems such as Altair charts not showing up with the default HTML renderer and SVGs are also not working, otherwise that would have been a simpler solution. I think it would be great to have a working ppi argument with vl-convert even in context that don't have access to a jupyter frontend (but I also recognize that this wouldn't be as much of an issue if SVG was working in Jupyter Book)

jonmmease commented 1 year ago

I tried using the branch in https://github.com/image-rs/image-png/pull/403 with vl-convert, and I was able to generate images with configurable DPI. Here are a few I generated:

72 PPI (default)

Screenshot 2023-07-17 at 1 10 11 PM

circle_binned

127 PPI

Screenshot 2023-07-17 at 1 11 01 PM

circle_binned copy 2

But the apple preview inspector is the only place I saw a difference in behavior. For example, GitHub renders them the same size.

I'm not sure what PNG rendering environments honor the PPI property. Have you looked into this? Do you want to try the two images above in Jupyter Book to see if they render with different sizes?

joelostblom commented 1 year ago

Hmm I don't see any difference with your two images but if I create two new images of 10 dpi and 100 dpi, I do see differences. Here are the two source images:

10 10

100 100

Here is how they show up in JupyterLab:

image

Here is how they show up in Jupyter Books PDF Latex output:

image

joelostblom commented 1 year ago

I'm not on a Mac so Ican't see what ppi is reported by Preview for those images but they seem to have the correct density based on running this command from the terminal:

identify -units PixelsPerInch -format '%[fx:int(resolution.x)]\n' 100.png

Although it seems like their reported size is also different, so I'm a bit confused about why both the size and the ppi differ.

I created these in seaborn via:

df = sns.load_dataset('iris')
sns_plot = sns.pairplot(df, hue='species', height=1)
sns_plot.figure.savefig('100.png',dpi=100)
jonmmease commented 1 year ago

Here's what these two look like in the preview inspector

10

Screenshot 2023-07-17 at 4 32 28 PM

100

Screenshot 2023-07-17 at 4 33 10 PM

It looks like preview doesn't care about PPI for rendering, it just displays based on pixel size

Screenshot 2023-07-17 at 4 41 02 PM
jonmmease commented 1 year ago

Just to make sure I'm following...

Based on your experiments above, it looks like Jupyter in the browser doesn't care about PPI, just pixels. But the PdfLatex output uses it to convert pixels to image size.

But, when you tried using my 76 and 127 PPI images with PdfLatex they both came out the same size in the resulting PDF? (I would have hoped that the 127 PPI image would turn out smaller).

joelostblom commented 1 year ago

But, when you tried using my 76 and 127 PPI images with PdfLatex they both came out the same size in the resulting PDF? (I would have hoped that the 127 PPI image would turn out smaller).

Yeah, this is odd, when I run you two images in jupyter book it looks like this:

image

What's also odd to me is that the smaller 72 ppi file seems larger at 19KB compared to the 127 ppi file (16KB). And when I run the identify command they both show 0 for PixelsPerInch and when I look at them by eye, I can't see a different in resolution. The comment here https://github.com/vega/vega-lite/pull/8854#issuecomment-1518451087 makes it seem like the PixelsPerInch (or Meter) needs to be set and it seems like it is maybe not set correctly on these images although the report a ppi?

jonmmease commented 1 year ago

I went back and regenerated three images more carefully. All 3 have scale=1 so all three have 360 x 247 pixels. The only thing that changes is the PPI:

10 ppi

circle_binned_10ppi

100 ppi

circle_binned_100ppi

200 ppi

circle_binned_200ppi

identify

The expected PPI shows up in the MacOS preview inspector and with the imagemagick identify CLI:

% identify -units PixelsPerInch -format '%[fx:int(resolution.x)]\n' ./circle_binned_10ppi.png 
10
% identify -units PixelsPerInch -format '%[fx:int(resolution.x)]\n' ./circle_binned_100ppi.png 
100
% identify -units PixelsPerInch -format '%[fx:int(resolution.x)]\n' ./circle_binned_200ppi.png 
200

Could you try running these three though Jupyter Book Latex PDF? We'd expect circle_binned_10ppi.png to be giant, circle_binned_100ppi.png to be about 3.6" across, and circle_binned_200ppi.png to be about 1.8" across.

If these all look the same in the LaTeX PDF output, I'm not sure what to try next 😕

joelostblom commented 1 year ago

Awesome thank you! These work as expected!!

image

joelostblom commented 1 year ago

So after https://github.com/image-rs/image-png/pull/403 has been merged, would vl-convert and/or altair need updates as well or does it pass through additional parameter so that the png renderer could take a ppi argument as is? I wonder if it would be confusing that the images don't change resolution in JupyterLab, but I guess we could remedy that by noting it in the documentation

jonmmease commented 1 year ago

Awesome thank you! These work as expected!!

🎉 I'll follow up on that PR. I don't have a good way of doing a full vl-convert release against a git branch of a dependency, but we can get started at least.

Would it be helpful to you to have a dev build ASAP for working on the book?

joelostblom commented 1 year ago

Great, thank you! Yes it would be helpful with a dev build for the book I'm working on, but I don't think it needs to be ASAP so whenever you get the chance to put one up then that's good for me. Thanks for exploring this and working out what was going on!

What do you think is the best solution for the fact that different contexts seem to handle interpret ppi attribute of PNG files differently? Both GitHub and JupyterLab seem to show the images as the same size, so is jupyter book the odd one out here? I guess since Jupyter supports SVG, it would be easy to recommend that format for seeing higher resolution image in jupyter book itself, and maybe this ppi option is mostly for saving/exporting image to PNG files as well as including them in contexts that don't support SVG, such as Jupyter Book with Latex

I asked a related question here https://github.com/jupyterlab/jupyterlab/issues/14840

jonmmease commented 1 year ago

Sounds good. I'll keep you posted.

Regarding displaying in Jupyter, I wonder if we could use a generalization of the retina trick above to achieve the same effect as what happens in Jupyter Book / LaTeX.

jonmmease commented 1 year ago

So after https://github.com/image-rs/image-png/pull/403 has been merged, would vl-convert and/or altair need updates as well or does it pass through additional parameter so that the png renderer could take a ppi argument as is?

Sorry, I missed this question. Yeah, vl-convert will need a new release to update this dependency and add the ppi kwarg to the vlc.*_to_png functions. Then we will need to update the PNG renderer in Altair to take a ppi argument and pass it through to vl-convert (if vl-convert is a new enough version). Ideally, the Altair PNG renderer would also include logic similar to the retina flag in IPython.display.Image to make the image appear to be the same size as it will in the PDF export, but I don't have a good sense yet of how complex that will be.