Support Pixel Ratio

[philipc2]

Closes #626

Overview

• Adds support for setting the pixel_ratio parameter

Example Plots

[philipc2]

@ahuang11

Thanks for pointing me in the right direction with #1368

This should be ready for review.

[ahuang11]

Thanks! Took a quick skim; could you also add a line to Customization.ipynb to document it's available?

[philipc2]

Thanks! Took a quick skim; could you also add a line to Customization.ipynb to document it's available?

Sure! Will run the pre-commit too

[ahuang11]

I think op_options is missing a comma. Otherwise I think it looks good and I can take a deeper look/merge tomorrow!

[ahuang11]

I applied a minor fix to Customization.ipynb; I believe it's ready!

[ahuang11]

Actually, the test seems to be failing:

    def test_pixel_ratio(self, da2):
        plot = da2.isel(other=0).hvplot(rasterize=True, pixel_ratio=4.0)
        opts = Store.lookup_options('bokeh', plot, 'plot')
>       assert opts.kwargs['pixel_ratio'] == 4.0
E       KeyError: 'pixel_ratio'

[ahuang11]

Pointed out by @maximlt, we should also mention this is applicable for datashader operations here and in the docstring.

Bonus if you also mention it for x_sampling/y_sampling!

[philipc2]

Done! Let me know what you think of the addition.

[philipc2]

Actually, the test seems to be failing:

    def test_pixel_ratio(self, da2):
        plot = da2.isel(other=0).hvplot(rasterize=True, pixel_ratio=4.0)
        opts = Store.lookup_options('bokeh', plot, 'plot')
>       assert opts.kwargs['pixel_ratio'] == 4.0
E       KeyError: 'pixel_ratio'

Do you have any suggestions on what I could do to fix this and/or how to best test this?

[ahuang11]

Thanks for updating customization! Can you also add to docstring?

For the failing test, can you see how datashading is asserted? (If there's a test for that)?

[philipc2]

Thanks for updating customization! Can you also add to docstring?

For the failing test, can you see how datashading is asserted? (If there's a test for that)?

Maybe @maximlt can provide some input, I'm not sure the best way to move forward with the test.

[ahuang11]

Let me investigate a bit too.

[ahuang11]

Okay I found it

import xarray as xr
import hvplot.xarray
import xarray as xr
import holoviews as hv

ds = xr.open_dataset("sst.oisst.mon.ltm.1991-2020.nc")["sst"].isel(time=0)
p = ds.hvplot.image(rasterize=True, pixel_ratio=0.1)
assert p.callback.inputs[0].callback.operation.p["pixel_ratio"] == 0.1

For future reference, I looked up how other datashading tests were done

Maybe we should migrate the test over from testoptions.py to testoperations.py too.

[philipc2]

Okay I found it

import xarray as xr
import hvplot.xarray
import xarray as xr
import holoviews as hv

ds = xr.open_dataset("sst.oisst.mon.ltm.1991-2020.nc")["sst"].isel(time=0)
p = ds.hvplot.image(rasterize=True, pixel_ratio=0.1)
assert p.callback.inputs[0].callback.operation.p["pixel_ratio"] == 0.1

For future reference, I looked up how other datashading tests were done

Maybe we should migrate the test over from testoptions.py to testoperations.py too.

Done! Moved them to testoperations.py and updated the tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.91%. Comparing base (efeda78) to head (fec2098).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1411      +/-   ##
==========================================
+ Coverage   88.73%   88.91%   +0.17%     
==========================================
  Files          51       51              
  Lines        7592     7651      +59     
==========================================
+ Hits         6737     6803      +66     
+ Misses        855      848       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maximlt]

Useful for higher resolution screens where the PlotSize stream reports 'nominal'
dimensions in pixels that do not match the physical pixels. For
instance, setting pixel_ratio=2 can give better results on Retina
displays.

My understanding is that currently Panel/HoloViews automatically pulls the right ratio based on the user display, so if that's the case this comment is misleading as e.g. users on Retina display will be led to set pixel_ratio=2 while this is already done internally.

Pasting a comment made by @philippjfr elsewhere:

That said I wonder if the pixel_ratio here should be relative to the display pixel ratio or simply override the existing pixel ratio as is done in the PR.

[philipc2]

Useful for higher resolution screens where the PlotSize stream reports 'nominal'
dimensions in pixels that do not match the physical pixels. For
instance, setting pixel_ratio=2 can give better results on Retina
displays.

My understanding is that currently Panel/HoloViews automatically pulls the right ratio based on the user display, so if that's the case this comment is misleading as e.g. users on Retina display will be led to set pixel_ratio=2 while this is already done internally.

Pasting a comment made by @philippjfr elsewhere:

That said I wonder if the pixel_ratio here should be relative to the display pixel ratio or simply override the existing pixel ratio as is done in the PR.

That may be the case, and I'm happy to rephrase it (took this word-for-word from the HoloViews source code)

[maximlt]

@ahuang11 I was reviewing this PR again and read through the original issue (failed to do that previously). I'd like to make sure we're going in the right direction as exposing pixel_ratio is not exactly what was suggested in the issue previously by @jbednar and discussed with @philippjfr and others. Well, one good reason for that is that pixel_ratio was only added in HoloViews after that discussion happened holoviz/holoviews#5288 🙃

Jim suggested 3 options, Philipp being happy with options 1 and 3.

1] hvPlot calls could accept an argument rasterize_args, a dictionary of key:value pairs that would be supplied as-is when hvPlot calls rasterize, after its own arguments. Pros: Provides the full power of hv.operation.datashader.rasterize(), including all the various arguments shown above, within the hvPlot syntax. Cons: Full usage requires looking up the HoloViews documentation for rasterize, at which point maybe people should just be using that operation directly.

3] hvPlot calls could accept an argument rasterize_factor that is multiplicatively applied to the plot's native height and width to get the rasterization resolution. Pros: One new argument rather than two. Makes it simple to express "I want a lower resolution than usual" without fiddling with specific widths and heights. Cons: Only addresses this one use case.

I like option 1 too and somewhat disagree with the cons cited by Jim, it seems to be hvPlot could document the main keys that can be set via this dict argument. Pretty sure there are ways to type this dictionary so that users would get help from their IDE to populate it (well not yet in Jupyter).

Option 3 is what I think is implemented with pixel_ratio? Except that in the issue there was a short discussion on finding a better name for rasterize_factor and I doubt pixel_ratio achieves that?

I would also like us to address this question before merging:

That said I wonder if the pixel_ratio here should be relative to the display pixel ratio or simply override the existing pixel ratio as is done in the PR.

[philipc2]

@maximlt

I can create a separate issue on this topic if you'd like,

As for the design, I'm open to any considerations.

[ahuang11]

Depends how useful pixel_ratio is; if it's really useful I vote for 3; if not 1.

Option 1 is similar to colorbar_opts; I shy away from having to call colorbar_opts because I never remember the proper kwarg values.

there are ways to type this dictionary so that users would get help from their IDE to populate it

Sounds like a dataclass / pydantic model / parameterized

[philipc2]

Depends how useful pixel_ratio is; if it's really useful I vote for 3; if not 1.

Option 1 is similar to colorbar_opts; I shy away from having to call colorbar_opts because I never remember the proper kwarg values.

there are ways to type this dictionary so that users would get help from their IDE to populate it

Sounds like a dataclass / pydantic model / parameterized

I've found pixel ratio to be extremely useful when paired with dynamic=False for interactive plots.

Zoomed (Default Pixel Ratio)

Zoomed (pixel_ratio=8.0)

It's generally pretty intuitive for generating higher-resolution raster plots, while maintain the same plot size.

[ahuang11]

I'm in favor of having pixel_ratio exposed directly.

[maximlt]

@philipc2 I took the liberty to push a commit with some changes to the docstring and re-ordering.

---

In the end, I decided that adding pixel_ratio wasn't making hvPlot's API so much worse:

• Keeping the same name as used in HoloViews has some value
• hvPlot's API is a bit of a mess anyway. For instance, you can pass max_px to configure dynspread, it's not in the signature or documented anywhere but is caught internally.

But in general @ahuang11 I think we need to be careful when adding new parameters to hvPlot, because of the flat namespace this adds a cost to all users of hvPlot.

[maximlt]

Thanks @philipc2 !