Setting expand to false for Image

[hoxbro]

Fixes #5358

To be honest, this fix is more of a workaround for the problem than a fix. But the complexity of the the holoviews pipelines makes it hard for me to see when self.p.expand is set to True, even though regrid.expand defaults to False. Suggestions are welcome.

With this change the notebook example outputs this:

screenrecord-2022-07-07_17.48.20.mp4

The example in the notebook can be reduced to this MRE:

import xarray as xr
import numpy as np
import holoviews as hv
from holoviews.operation.datashader import rasterize

da = xr.DataArray(
    np.arange(10_000).reshape(100, 100),
    coords=dict(
        x=np.linspace(166021, 534994, 100),
        y=np.linspace(0.00, 9329005, 100),
    ),
)
rasterize(hv.Image(da))

Before this fix:

screenrecord-2022-07-07_17.54.46.mp4

After this fix:

screenrecord-2022-07-07_17.52.09.mp4

[codecov-commenter]

Codecov Report

Merging #5351 (ac300a4) into master (5ce1e1d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5351   +/-   ##
=======================================
  Coverage   88.07%   88.07%           
=======================================
  Files         301      301           
  Lines       61957    61959    +2     
=======================================
+ Hits        54567    54569    +2     
  Misses       7390     7390           

Impacted Files	Coverage Δ
holoviews/operation/datashader.py	84.01% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ce1e1d...ac300a4. Read the comment docs.

[hoxbro]

A comment about why this is done is needed before merging into master.

[jbednar]

Hmm. It's good that this fixes the symptom, but I agree it seems like a workaround, because expand shouldn't affect whether the plot works ok; it should only affect whether the image is regridded into the entire viewport or only into the original extent of that image. Both options should result in the same visual display for the default transparency and other settings, so if the image renders only for one of the settings, I think that's a bug rather than an indication that we should use that setting.

[hoxbro]

This workaround works because it avoids setting x_range = self.p.range in the following lines but instead calculates them based on the element range. Where self.p.range is the plot's range, which changed when moving around the plot.

holoviews/holoviews/operation/datashader.py

Lines 145 to 154 in 2d2b728

	elif self.p.expand or not self.p.x_range:
	if self.p.x_range and all(isfinite(v) for v in self.p.x_range):
	x_range = self.p.x_range
	else:
	x_range = max_range([element.range(xd) for xd in x])
	else:
	x0, x1 = self.p.x_range
	ex0, ex1 = max_range([element.range(xd) for xd in x])
	x_range = (np.nanmin([np.nanmax([x0, ex0]), ex1]),
	np.nanmax([np.nanmin([x1, ex1]), ex0]))

Same thing with y_range.

I tried always calculating the range, but this made the tests fail.

[jlstevens]

I've discussed this with @hoxbro and I see a number of overlapping issues:

1. It is possible I am forgetting something, but where does the fill value come from when expanding? This should be settable but I don't see where to do that right now (if so, the docstrings need improving!). Of course, for images you can control the visual display at the colormapping level but you should still have this control over the data generated...
2. You can't specify the transparent value when color mapping for RGBs as RGBs aren't color mapped. I would argue that regrid (or rasterize in hvplot) for RGBs should use white+transparent (0% alpha) in the region where there is no data. I suppose this could also be exposed as an RGB specific option where you specify an RGB or RGBA tuple...
3. Is expand an option that should be exposed at the hvplot level and I'm just wondering if the default of expand=True was an explicit decision with a good rationale?

Once we are clear on these issues, then it should become more obvious what the correct fix is...

Edit: I'm remembering a bit more now, I believe that the black for NaNs comes back to an old convention in datashader, in which case matching that for missing values made some sense...but transparent pixels are an equally valid and useful choice! Do you agree @jbednar ?

[jbednar]

I agree. I don't remember all the details, but would have thought we always preferred transparent for NaN, albeit with black as the color, which wouldn't normally be visible.

[hoxbro]

Superseded by #5767

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.