-
Notifications
You must be signed in to change notification settings - Fork 793
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
Add ppi argument for saving and displaying charts as PNG images #3163
Conversation
@joelostblom Could you try LaTeX to PDF output with this branch, vl-convert-python 0.13.0, and with image rendering enabled with Also, if you have a chance, would you mind taking pass at explaining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Latex PDF output seems to work! Thank you!
Note that when I run it with an older version of altair, it runs without an error but produces the wrong output (only scales but does not adjust the ppi). I wonder if this can be confusing, making it seem like the ppi
option is not working. Do you think we could add a check for if the ppi
parameter is used with an incompatible version of vl-convert
/altair
and raise a warning/error?
Also note the warning in the topmost screenshot regarding anywidget. Should we silence that since it might be confusing for users not knowning that they are using naywidget with latiar?
@manzt Is there something we should be doing to avoid the "Live reloading is disabled..." warning when not in development mode? |
Thanks for trying it out @joelostblom, glad it's working! Regarding compatibility. Altair 5.1 will require vl-convert 0.13.0 which has ppi support. I don't think there's anything we can do retroactively to warn users of Altair 5.0.1 that ppi isn't supported unfortunately. Does that make sense? |
altair/utils/mimebundle.py
Outdated
) | ||
return {"image/png": png} | ||
factor = ppi / 72 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment why ppi is devided by 72?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, also tested it locally. Only one minor request in the code and also could you add a line to the changelog? Thank you @jonmmease! This is very handy.
+1 for silencing the warning |
Hi all, thanks for pinging me. The "watch mode" is automatically enabled if the widget file is outside of "site-packages" (e.g., a development install). There currently isn't a way to disable this behavior (i.e. starting a watch thread for a file outside of import os
os.environ["ANYWIDGET_WATCH"] = True
class MyWidget(anywidget.AnyWidget):
... |
Thanks @manzt, the site-packages convention makes sense. I'll add |
@joelostblom and @binste, I added a changes entry and updated the save chart documentation to discuss |
Agree. Thanks @manzt for the fast response! I was worried that users would see this but this makes sense.
Looks good, thanks Jon! |
I get the following error when running the altair specification. File C:\Notebooks\altair\altair\altair\utils\mimebundle.py:136, in _spec_to_mimebundle_with_engine(spec, format, mode, **kwargs)
130 png = vlc.vega_to_png(
131 spec,
132 scale=scale,
133 ppi=ppi,
134 )
135 else:
--> 136 png = vlc.vegalite_to_png(
137 spec,
138 vl_version=vl_version,
139 scale=scale,
140 ppi=ppi,
141 )
142 factor = ppi / default_ppi
143 w, h = _pngxy(png)
TypeError: vegalite_to_png() got an unexpected keyword argument 'ppi' Is this possible? I assumed |
Oh, good catch! We're not using |
This PR adds support for a new
ppi
argument to control the pixels-per-inch metadata of the PNG files generated by vl-convert 0.13.0.For background discussion on ppi see:
Saving PNG files with ppi
This will produce a file named
box.png
that is scaled to be twice as large as the original chart, with a ppi of 300. Not all image viewers respect PPI when displaying PNG images, but Preview on MacOS at least shows the ppi metadata in the info panel:Display as PNG
Chart's can be displayed as PNG images with
LaTeX
When exporting notebooks to PDFs with LaTeX the ppi metadata is used to determine the physical size that the image should be displayed at.
Jupyter
Jupyter / Browsers don't seem to use the png image's ppi metadata to determine the size of the output image. To get this behavior, we can follow the logic that the
IPython.display.Image
uses when theretina
argument is set toTrue
. I followed the implementation in https://github.com/ipython/ipython/blob/main/IPython/core/display.py#L809 to learn how to control the displayed width/height of a PNG mime bundle in Jupyter.This involved swiping the
_pngxy
function to extract the image width/height from the png image bytes returned by vl-convert.Here's what this looks like in JupyterLab: