holoviews
holoviews copied to clipboard
Setting expand to false for Image
Fixes https://github.com/holoviz/holoviews/issues/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:
https://user-images.githubusercontent.com/19758978/177816756-b6749b29-282f-4097-a1ae-daf4a7eed9a2.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:
https://user-images.githubusercontent.com/19758978/177817920-cb102b31-8887-4b1f-9e78-cfe77f92c334.mp4
After this fix:
https://user-images.githubusercontent.com/19758978/177817400-b2837463-019d-4ead-90cd-cd54c840e7a9.mp4
Codecov Report
Merging #5351 (ac300a4) into master (5ce1e1d) will increase coverage by
0.00%
. The diff coverage is100.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%) |
:arrow_up: |
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.
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.
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.
https://github.com/holoviz/holoviews/blob/2d2b72809eb2e2d75f1e26f3fad97892b2d6a720/holoviews/operation/datashader.py#L145-L154
Same thing with y_range
.
I tried always calculating the range, but this made the tests fail.
I've discussed this with @Hoxbro and I see a number of overlapping issues:
- 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...
- You can't specify the transparent value when color mapping for RGBs as RGBs aren't color mapped. I would argue that
regrid
(orrasterize
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... - Is
expand
an option that should be exposed at thehvplot
level and I'm just wondering if the default ofexpand=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 ?
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.
Superseded by https://github.com/holoviz/holoviews/pull/5767