Skip to content
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

Plotting with a discrete nonlinear colorbar assigns values to colors incorrectly #5966

Closed
JulianGiles opened this issue Oct 30, 2023 · 12 comments
Labels
type: bug Something isn't correct or isn't working

Comments

@JulianGiles
Copy link
Contributor

ALL software version info

Name Version Build Channel
holoviews 1.17.1 pyhd8ed1ab_0 conda-forge
hvplot 0.8.4 pyhd8ed1ab_1 conda-forge
bokeh 3.2.2 pyhd8ed1ab_0 conda-forge
xarray 2023.9.0 pyhd8ed1ab_0 conda-forge
python 3.11.5 hab00c5b_0_cpython conda-forge
spyder 5.4.5 py311h38be061_0 conda-forge
spyder-kernels 2.4.4 unix_pyh707e725_0 conda-forge

Description of expected behavior and the observed behavior

I am trying to plot a QuadMesh with a discrete nonlinear colorbar. I have found help from several posts in the discourse and GitHub but mainly this one. When plotting by passing a discrete list of colors (cmap) and levels (color_levels), a plot is produced which looks correct at first glance. But, on closer inspection or when the data is more challenging (more extreme values or very irregular color ranges) some values on the verge of a color level are incorrectly assign to a different color.

Complete, minimal, self-contained example code that reproduces the issue

import xarray as xr
import hvplot.xarray
from bokeh.models import CategoricalColorMapper, ColorBar

COLORS = ["magenta", "green", "yellow", "orange", "red", "purple"]
BOUNDS = [0, 230, 250, 270, 300, 320, 400]

# define a function to plot a colorbar with equally-sized color bins
def cbar_hook(hv_plot, _):
    plot = hv_plot.handles["plot"]
    factors = [f"{BOUNDS[i]} - {BOUNDS[i + 1]}" for i in range(len(COLORS))]
    mapper = CategoricalColorMapper(
        palette=COLORS,
        factors=factors,
    )
    color_bar = ColorBar(color_mapper=mapper)
    plot.right[0] = color_bar


ds = xr.tutorial.open_dataset("air_temperature")
ds.hvplot.quadmesh("lon", "lat").opts(
    cmap=COLORS,
    color_levels=BOUNDS,
    clim=(BOUNDS[0], BOUNDS[-1]),
    hooks=[cbar_hook],
)

In this example, you can also change the extreme values in BOUNDS to be more extreme and the problem is more notorious.

Screenshots or screencasts of the bug in action

image
See example of a value that should be colored yellow but it is green.

  • [ x] I may be interested in making a pull request to address this
@ahuang11
Copy link
Collaborator

ahuang11 commented Oct 30, 2023

I suspect some rounding issue is going on here: https://github.com/holoviz/holoviews/blob/main/holoviews/plotting/util.py#L954-L1000

Maybe it needs to use np.floor or np.ceil

@philippjfr philippjfr added the type: bug Something isn't correct or isn't working label Oct 30, 2023
@JulianGiles
Copy link
Contributor Author

JulianGiles commented Oct 31, 2023

Just changing round for np.floor or np.ceil does not solve the issue. I do not understand completely how the colors are used. color_intervals() returns the input colormap but mapped to 255 values. Why does it have to be 255? I think being able to pass a custom norm argument could help fix this problem but it is not a valid option when plotting with bokeh backend and it does not work with matplotlib backend either (I posted an issue a while ago here).

@JulianGiles
Copy link
Contributor Author

Increasing the default value of N in color_intervals() from 255 to 1000 or 10000 works to solve the issue. Basically it is increasing the resolution of the colormap. But it will be dependent on the color_levels provided, if this increased resolution is enough to catch the precision needed. So I think it is still a sub-optimal solution.

@JulianGiles
Copy link
Contributor Author

JulianGiles commented Oct 31, 2023

The "fix" of increasing N does nothing if using matplotlib backend. Again, this may be solved by being able to pass norm to the plotting call, but there is a problem. It could be solved by adding:

if 'norm' in plot_kwargs: # vmin/vmax should now be exclusively in norm
     plot_kwargs.pop('vmin', None)
     plot_kwargs.pop('vmax', None)

to the definition of init_artists inside class QuadMeshPlot in line 192 in holoviews/plotting/mpl/raster.py, in the same way this norm condition is already present in line 594 in /holoviews/plotting/mpl/element.py

@ahuang11
Copy link
Collaborator

ahuang11 commented Oct 31, 2023

Thanks for investigating. I think it's still worthwhile to deploy a fix for bokeh.

I'd support adding support for norm to mpl, maybe as a norm = param.String() and implement under _norm_kwargs?
#5883

@JulianGiles
Copy link
Contributor Author

Thanks for investigating. I think it's still worthwhile to deploy a fix for bokeh.

I'd support adding support for norm to mpl, maybe as a norm = param.String() and implement under _norm_kwargs? #5883

In my case the mpl backend is working correctly when passing norm to the plot call directly or inside .opts() (after applying the fix that I just posted above). Am I missing something?

@ahuang11
Copy link
Collaborator

ahuang11 commented Oct 31, 2023

Ooh I see; I needed to pass norm to opts not colorbar_opts. Thanks for sharing.

import holoviews as hv
import pandas as pd
import matplotlib as mpl

df = pd.DataFrame(
    {
        "x": range(1, 9),
        "y": [1] * 8,
        "z": range(1, 9),
    }
)

hv.extension("matplotlib")

bounds = [1, 6, 7, 9]
cmap = mpl.colors.ListedColormap(["red", "green", "blue"])
norm = mpl.colors.BoundaryNorm(bounds, cmap.N)

hv.Scatter(
    df,
    kdims=["x", "y"],
    vdims=["z"],
).opts(
    colorbar=True,
    cmap=["red", "green", "blue"],
    norm=norm,
    colorbar_opts={
        "ticks": bounds,
        "fraction": 0.6,
    },
    s=120,
    c="z",
)
image

@JulianGiles
Copy link
Contributor Author

Passing norm to opts is enough (at least for me).

@ahuang11
Copy link
Collaborator

Updated!

@JulianGiles
Copy link
Contributor Author

The fix regarding passing 'norm' when plotting a quadmesh with matplotlib backend has been merged into the main branch. The issue on how to handle the same problem with bokeh remains. I still don't know what the best approach could be.

@hoxbro
Copy link
Member

hoxbro commented Jun 24, 2024

I think this should have been fixed by #4898

image

I will close this issue, but you are welcome to reopen it if you don't think the problem is solved.

@hoxbro hoxbro closed this as completed Jun 24, 2024
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't correct or isn't working
Projects
None yet
Development

No branches or pull requests

4 participants