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

hv.Image(group="") no longer works #5130

Closed
jbednar opened this issue Nov 12, 2021 · 5 comments · Fixed by #5131
Closed

hv.Image(group="") no longer works #5130

jbednar opened this issue Nov 12, 2021 · 5 comments · Fixed by #5131
Labels
TRIAGE Needs triaging

Comments

@jbednar
Copy link
Member

jbednar commented Nov 12, 2021

Until HoloViews 1.13.5 it was legal to use a group name of "" for an Image, but starting with v1.14.0 that gives an error:

import numpy as np, holoviews as hv
hv.extension()

ls = np.linspace(0, 10, 200)
xx, yy = np.meshgrid(ls, ls)
hv.Image(np.sin(xx)*np.cos(yy), group="").opts(cmap="greys")

image

In a brief investigation I couldn't see what code had changed, but apparently something has. In any case, searching for "capitalize" in https://github.com/holoviz/holoviews/blob/master/holoviews/core/util.py reveals various code paths that implicitly depend on the length of a string being nonzero, which doesn't seem like a safe assumption in general and is problematic in this case.

Here, obviously group could simply be omitted, but this came up for existing code in colorcet that accepted a user argument for the group, and when the user wants a blank group they now have to set the group to the very non-intuitive value "Image", whereas the much more reasonable group="" was sufficient to suppress printing the group name before.

@jbednar jbednar added the TRIAGE Needs triaging label Nov 12, 2021
@hoxbro
Copy link
Member

hoxbro commented Nov 13, 2021

A git bisect shows that this was introduced in #4567

@jbednar
Copy link
Member Author

jbednar commented Nov 13, 2021

Thanks; that helps! Alas, #4567 is a squash of 98 commits, with changes to 51 files, so we'll probably have to git bisect within that PR.

@hoxbro
Copy link
Member

hoxbro commented Nov 13, 2021

Done. Commit 969aa17 seem to be the culprit.

You can get the desired plot by adding nodata=True in opts.

import numpy as np
import holoviews as hv
import panel as pn

hv.extension()

ls = np.linspace(0, 10, 200)
xx, yy = np.meshgrid(ls, ls)
image = hv.Image(np.sin(xx) * np.cos(yy)).opts(cmap="greys", nodata=True)

@jbednar
Copy link
Member Author

jbednar commented Nov 13, 2021

Thanks! That commit doesn't seem to change any of this behavior directly, but it must have somehow triggered it, e.g. by defining a new Compositor for Images. #5131 seems to fix it by addressing the underlying assumption that a string will always have a first character, which is not true in this case.

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
TRIAGE Needs triaging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants