holoviz / holoviews

With Holoviews, your data visualizes itself.
https://holoviews.org
BSD 3-Clause "New" or "Revised" License
2.7k stars 402 forks source link

Layout doesn't scale with VLines #3759

Closed cristi-neagu closed 5 years ago

cristi-neagu commented 5 years ago

Hello,

I'm using this example:

# Generate sample data
mu, sigma = 0, 0.1
s = np.random.normal(mu, sigma, 1000)

dist = hv.Histogram(np.histogram(s, bins=20))
s3min = hv.VLine(mu - sigma*3)
s3max = hv.VLine(mu + sigma*3)
s6min = hv.VLine(mu - sigma*6)
s6max = hv.VLine(mu + sigma*6)
layout = dist * s3min * s3max * s6min * s6max
layout

The layout doesn't extend to show all the lines. The X axis only scales to show the histogram. If i overlay another distribution, it does extend, but it's like it doesn't take VLines into account when calculating the bounds.

jbednar commented 5 years ago

Right; VLines are annotations, and annotations are not taken into account when calculating the bounds. That's because annotations aren't considered data. E.g. one can have a set of helpful annotations that indicate global stock market events, overlaid with actual daily stock data from a variety of time periods, and only the actual data will be used to determine the plot bounds; annotations will then appear if they are in the bounds, but otherwise won't affect the plot.

If you want to treat vertical lines as data, see the Spikes element. But note that Spikes are finite in height, and you might want to treat the x positions as data while having the y position extend infinitely and scale to some other dataset; that "in-between" behavior of having one axis be data-like and the other be annotation-like isn't currently supported.

cristi-neagu commented 5 years ago

I was afraid that's why it doesn't work... Well, spikes aren't a real alternative right now exactly because they don't fill the entire height of the plot, and require a work around. And I don't think they work for horizontal lines.

Unfortunately I rely heavily on horizontal and vertical lines to put limits and thresholds on my data. These two limitations combined (lines aren't data and spikes are finite) makes it too much of a hassle for me to use Holoviews. Which is a shame, cause otherwise it is great.

I'm closing this since from what I could understand from other issues on here lines will most likely never be considered data, and the update to make spikes infinite hopefully will be delivered in v2.0 (which sounds like a very long time into the future, considering we are at v1.12.3)

Thank you.

philippjfr commented 5 years ago

I think we could definitely consider the idea that annotations simply set apply_range=False by default and let the user override the behavior. That wouldn't be much work and provide sufficient flexibility while not affecting current behavior.

jbednar commented 5 years ago

Spikes work fine for horizontal lines, btw:

import numpy as np
import holoviews as hv
from holoviews import opts
hv.extension('bokeh')

xs = np.random.rand(50)
spikes = hv.Spikes(xs)
spikes.opts(line_alpha=0.4, spike_length=0.1).opts(invert_axes=True)
Screen Shot 2019-06-06 at 8 32 52 AM

I'm not sure if there is a more elegant way to do that.

philippjfr commented 5 years ago

That inverts the whole axis, there probably should be distinct option that inverts just the one element.

poplarShift commented 5 years ago

@cristi-neagu Somewhat more verbose, but the Segments element: https://github.com/pyviz/holoviews/pull/3532 may be able to do what you want.

cristi-neagu commented 5 years ago

@cristi-neagu Somewhat more verbose, but the Segments element: #3532 may be able to do what you want.

@poplarShift Looks like Segments don't have infinite length either, from what i see.

I think we could definitely consider the idea that annotations simply set apply_range=False by default and let the user override the behavior.

@philippjfr Is there a way i can force the layout to take annotations into account when setting bounds, or are you talking about new functionality?

jbednar commented 5 years ago

There probably should be distinct option that inverts just the one element.

That was what I was thinking as well, but it's tricky because effectively horizontal spikes are using the y axis as the independent axis, which is weird if you're overlaying it on something like Points where the x axis is the independent axis. But because Spikes can have two independent axes (if the lengths are data), then it can make perfect sense in some cases, so it's tricky. My guess is that Spikes should have a constructor argument (not via .opts) to be laid out along the y axis; it's not really a styling issue but a semantic one. But others may have other opinions and/or a deeper understanding of it.

jbednar commented 5 years ago

Is there a way i can force the layout to take annotations into account when setting bounds

I don't know what that would mean, given that VLines are infinite in extent. Do you mean, take the x location of the VLine into account, while ignoring the infinite extent?

cristi-neagu commented 5 years ago

Is there a way i can force the layout to take annotations into account when setting bounds

I don't know what that would mean, given that VLines are infinite in extent. Do you mean, take the x location of the VLine into account, while ignoring the infinite extent?

Good question, didn't actually think about the infinite length. Yes, only the X axis would rescale, ignoring the infinite length. This would obviously be reversed for HLines.

jbednar commented 5 years ago

And I think that's precisely the tricky semantics that's waiting on hv 2.0, i.e. things like true support for categorical x axis but numerical y axis, or finite x values used in ranges but infinite y values. I don't know all the details, but the current architecture definitely has some constraints like that and it's difficult to overcome them immediately. So far people have been able to work around using finite lines; certainly not every toolkit even offers infinite lines!

poplarShift commented 5 years ago

I thought I'd toy around with this so I reopened and made a PR.

cristi-neagu commented 5 years ago

Looks like this isn't going to get progressed, unfortunately...

poplarShift commented 5 years ago

@cristi-neagu You should have some patience. Normally if you want things to happen really urgently, you have to do them yourself. In this case, I even submitted a PR, but core devs have free time and priorities too...

(In the present case while the behaviour you want would be "nice to have", I really don't see why you can't work around it by setting .redim.range manually....)

cristi-neagu commented 5 years ago

@poplarShift I saw you PR, and i thank you for that, but i see it failed unit tests, so i assumed it's a no go. Is that assumption wrong?

cristi-neagu commented 5 years ago

Thank you very much.