Box points hover & select #2094

etpinard · 2017-10-17T03:41:41Z

resolves #192

In more detail, this PRs:

adds support for dragmode lasso and select to box traces

adds hoveron attribute the box traces with two available flags: boxes (the current behavior) and points:

adds text attribute the box traces so that custom hover text can be added to each box point.

- generate 'pts' array of objects similar to scatter pts inside box calcdata items, instead of simply keep track of all values corresponding to each box. - fill in pt objects fill w/ jitter during Box.plot instead of mapping box 'val' array to d3-esque array of objects. - in preparation for box select and 'points' hover, keep track of original val indices in pt object

- use pts array of object to determine which box points are inside selection polygon - use index field (from Box.calc) to match each selected box point to underlying x/y input value. - dim point node opacity upon selection

- flaglist w/ 'boxes' (current + default) and 'points' - use point object x/y values (from Box.plot) to determine closest point(s) - consider jitter offset under hovermode closest, but don't under 'x' or 'y' modes

- associated with each sample pt - only visible on hover (for now)

etpinard · 2017-10-17T03:47:38Z

src/traces/box/calc.js

+
+        var n = Lib.findBin(pos[i], posBins);
+        if(n >= 0 && n < pLen) {
+            var pt = {v: v, i: i};


This here was the key to make box point hover and selection somewhat clean.

Box calcdata traces have one item per box to be displayed. Box points data-esque array of objects used to be created during Box.plot, now the structure is setup here. Note that it is important to track the original sample pt index (that i above) to convert hovered are selected calcdata items to event data pointNumber corresponding to indices in the input x/y sample arrays

... probably best to review this file as a whole:

https://github.com/plotly/plotly.js/blob/6a878b0eef3e52332bd84eca40d024eec1219264/src/traces/box/calc.js

Definitely nice and clean - I was curious whether this degraded performance for large data sets, but it looks like very little if at all

function r(n) { var v = 0; for(var i = 0; i < n; i++) v += Math.random(); return v; } function a(n, m) { var out = new Array(n); for(var i = 0; i < n; i++) out[i] = r(m); return out; } var y = a(1000000, 10) // I had to patch timeit to support n = 1 timeit(function() { Plotly.newPlot(gd,[{type: 'box', y: y, jitter: 0.5, hoveron: 'points'}]) }, 1)

On my computer 1 million points (of which ~5k are outliers) takes ~3 sec, and difference between this branch and master is within the noise (~10% or less)

(~10% or less)

Thanks for taking a look at this 🐎

etpinard · 2017-10-17T03:48:51Z

src/traces/box/attributes.js

+    hoveron: {
+        valType: 'flaglist',
+        flags: ['boxes', 'points'],
+        dflt: 'boxes',


Debatable choice here. Strictly speaking we need dflt: 'boxes' to keep backward compat, but perhaps someone can argue otherwise.

dflt: 'boxes' 👍

I agree here, but perhaps to make this feature accessible to existing graphs, the show closest data on hover mode bar button could restyle hoveron to 'points+boxes' as well as layout.hovermode on click for traces with boxpoints: 'all'?

oh, hmm... that's a good point actually. Can I change my answer to dflt: 'boxes+points'? I don't think turning on a new hover feature really amounts to a breaking change, I suppose in principle it can lead to events that a user application doesn't know how to handle but that seems pretty minor IMHO.

dflt: boxes+points 👍 One could even argue that not being able to hover on the box points is a 🐞 .

done in 6bfcbe0

etpinard · 2017-10-17T03:52:24Z

src/traces/box/hover.js

+            }
+        }
+    }
+
    return closeData;


Another debatable choice: this commit allows 'boxes' and 'points' hoveron picks to coexist. With hoveron: 'boxes+points', we get:

where box and point hover label can be displayed at the same time. Does anyone find this potentially problematic?

... and under hovermode: 'x', this can happen:

boxes+points could be useful, but I think we want to show fewer points. Currently it seems to return all the points that are in range, but most real-world box uses have far too many points to label them all, and we should match scatter which chooses the best point when more than one is in range.

So I guess I'm proposing:

In closest mode, show only one point or stats for one box, and points have priority

If there's a point in range and hoveron has points, show the best single point only.

If hoveron has boxes and there's no point in range (or hoveron doesn't have points), show the box stats.

In compare mode I don't really see any extra useful thing to do with points - I think we should still only show one point per trace, so I guess that means not considering only x when choosing the best point? I know that's not consistent with other types, but I feel like usability is much more important.

Perhaps though if hoveron = 'boxes+points' we should allow a point AND the box stats to be labeled?

Already if there are multiple boxes in range (ie boxmode = 'overlay') we'll see stats for all of them, right? I think that's good.

One other thought: do we want to do something to distinguish statistics labels from point labels? Perhaps actually say what statistical measure is being shown? Which might be nice on its own merits! But we'd have to be careful about the whisker ends, whether they represent upper/lower fence or max/min.

but most real-world box uses have far too many points to label them all, and we should match scatter

Good point here. I'll implement ⤴️

Here's what I come up with: f228a5e

One other thought: do we want to do something to distinguish statistics labels from point labels? Perhaps actually say what statistical measure is being shown?

The above commit does not implement this. I think it's a good idea though. Should the statistical measures always show up next to their value in the hover labels or just when point and box hover label show simultaneously?

Here's a quick-and-easy version:

we might have to expand out uf and lf. Spelling out upper fence and lower fence looks like:

does that look too cluttered to you?

👍 for spelled out, I think you're right that uf / lf will confuse people.

maybe even spell out median

full version:

done in a3ec75a (with 🔨 🌴 commit b66628e)

etpinard · 2017-10-17T03:52:55Z

src/traces/box/hover.js

+        var xPx = xa.c2p(xval);
+        var yPx = ya.c2p(yval);
+
+        // do not take jitter into consideration in compare hover modes


Anyone disagrees here?

I agree, and might even take it farther - can we make all of the hover labels line up horizontally, even though the points do not?

oh hmm I guess https://github.com/plotly/plotly.js/pull/2094/files#r145223324 supersedes this comment, except perhaps if we show a point and stats in compare mode.

etpinard · 2017-10-17T03:54:46Z

src/traces/box/select.js

+    }
+
+    node3.selectAll('.point').style('opacity', function(d) {
+        return d.dim ? DESELECTDIM : 1;


Note that this style update isn't compatible with the current trace attribute set.

To do so, we would need to add arrayOk support for marker.color, marker.opacity and marker.symbol would be pretty easy to add after d352fa5

... that is if no one finds too awkward to have both box-wide (e.g. marker.outliercolor) and per-sample-pt attributes in the same trace.

Seems reasonable. outliercolor is only for boxpoints: 'suspectedoutliers', and in that mode selecting would be a bit weird. (side note: outliercolor should probably be described better in attributes... and perhaps the logic in defaults cleaned up or amended so we don't have the outliercolor attributes in _fullData if boxpoints !== 'suspectedoutliers'

Begs the question though of what selection (or crossfilter) should do in modes other than boxpoints: 'all' when at least the points within the box are hidden - what does selection do now? I could imagine it being cool to still be able to select the points, and have them pop into view when you do (likewise for crossfilter)... but that might be too complicated, dunno.

Begs the question though of what selection (or crossfilter) should do in modes other than boxpoints: 'all' when at least the points within the box are hidden - what does selection do now?

I'd vote for hiding the lasso and select mode bar buttons whenever boxpoints !== 'all' - similar to how we currently hide them from scatter-line-only graphs here. But maybe that's just because I'm lazy. cc @chriddyp @cpsievert any thoughts on this?

and perhaps the logic in defaults cleaned up or amended so we don't have the outliercolor attributes in _fullData if boxpoints !== 'suspectedoutliers'

I believe that's done already.

I'd vote for hiding the lasso and select mode bar buttons whenever boxpoints !== 'all'

We can certainly start that way anyhow, that's fine for this PR, we can revisit if there's interest later.

I believe that's done already.

It's messy logic - but that's actually the second coerce of one of them so the easy thing to do would be to delete them from the full trace if they're not needed. I guess the reason it's done this way is so you can set the default boxpoints to 'suspectedoutliers' if either of the outliercolor values is a real color - stricter than just existing.

Ha I see. I've been wanting to replace Lib.coerce2 with Lib.validate (which doesn't fill in fullData). I'll do this in this PR 🔨

🔪ing Lib.coerce2 turned out to be a little trickier than I thought - and than I want to do in this PR. In brief, we can just replace coerce2 by validate, validate return false whenever an value isn't set while coerce2 returns the default value.

So, here's a quick 🛠️ and 🔒 in 9ca410a

I'd vote for hiding the lasso and select mode bar buttons whenever boxpoints !== 'all'
We can certainly start that way anyhow, that's fine for this PR, we can revisit if there's interest later.

that's in b4d8044

etpinard · 2017-10-17T03:56:23Z

src/traces/box/attributes.js

@@ -62,6 +62,16 @@ module.exports = {
            'missing and the position axis is categorical'
        ].join(' ')
    },
+    text: extendFlat({}, scatterAttrs.text, {


text is expected to be an array as long as the sample array y (x) for vertical (horizontal) box plots, matching each sample pt one-to-one.

etpinard · 2017-10-17T04:02:17Z

test/jasmine/tests/select_test.js

+                        [2, 0.3], [2, 0.6], [2, 0.6]
+                    ]);
+                    assertLassoPoints([
+                        ['day 1', 'day 2', 'day 2', 'day 1', 'day 1'],


@alexcjohnson Is this the desired behavior, or did an extra c2d conversion get forgotten? I believe outputting the linearized continuous category equivalent would make sense here.

... we might have to wait for v2 though.

I guess c format would make sense for applications that want to perhaps overlay something on the selected points... but I'd think the d value is what the user would expect (and what they would generally want to use), wouldn't it?

My argument for c:

d is lossy for categories. Converting c to d can easily be done by the user, the opposite cannot (unless we officially expose ax.d2c and friends).

So do you think the x/y coordinate should be converted to d too here?

d is lossy for categories.

These values had really better correspond to the actual data, not offset or jittered values... It's not important for category axes, but for non-lossy axes it is, otherwise you'll be pointed to the wrong data value! So in the end I don't think that this conversion is lossy, even though in general you're right.

Converting c to d can easily be done by the user, the opposite cannot (unless we officially expose ax.d2c and friends).

How do you convert c to d for categories without ax.c2d? It depends on all sorts of details of the graph. Granted enough people use those methods already that I think we have to consider them exposed...

So do you think the x/y coordinate should be converted to d too here?

Good catch - yeah, I guess so. Everything we report back out with events should be d, I would think. That's a pretty sweeping statement though, so it's worth discussing.

How do you convert c to d for categories without ax.c2d?

Yeah, I guess that's kinda hard to in general. Put that in the 9am brain-cramp bin.

Everything we report back out with events should be d

I think so too. We should at the very least report everything in the same coordinate space. Right now, lasso coords are in d but point coords in are c even for scatter - which is ridiculous. I would consider this a bug, do you agree?

I would consider this a bug, do you agree?

🐞
I hope not too many people depend on the current behavior with date axes, but yeah, I think we should unify on d ASAP. I guess the lesson is make sure there are tests on non-numeric axes whenever we add new features like this... otherwise the c/d distinction goes unnoticed.

done in 8901528

alexcjohnson · 2017-10-17T15:04:30Z

src/traces/box/attributes.js

+        editType: 'style',
+        description: [
+            'Do the hover effects highlight individual boxes ',
+            'or jitter points or both?'


I wouldn't call them jitter points - sample points?

Good eye. Thanks!

done in a1ea1e8

- delete 'marker' container for box trace that don't have `boxpoints: false` - do not coerce 'text' when `boxpoints: false`

- show only 'best' boxpoint - consider opposite coordinates in compare modes to find that 'best' boxpoint - allow box AND boxpoint hover labels only in compare modes - no need for x/y w/o jitter offset values in 'pts' calc items

alexcjohnson · 2017-10-19T15:44:22Z

src/traces/scattercarpet/select.js

@@ -24,7 +24,6 @@ module.exports = function selectPoints(searchInfo, polygon) {
        cdi = cd[pt.pointNumber];
        pt.a = cdi.a;
        pt.b = cdi.b;
-        pt.c = cdi.c;


haha right, more ternary cruft 😅

- dry special log-off-scale hover label text routine - use it in hover.js cleanPoint - allow hoverPoints modules to set their own xLabel and yLabel value

alexcjohnson · 2017-10-19T18:15:51Z

Looks great! 💃

yniknafs · 2018-04-10T18:03:51Z

@etpinard @alexcjohnson

Curious if this has been implemented in the newest version of plotly. Not sure if I'm missing something, but in this pen, the hoverinfo: text option is not working.

https://codepen.io/yniknafs/pen/KoJOwQ

etpinard · 2018-04-10T20:55:35Z

@yniknafs your link is working for me:

yniknafs · 2018-04-10T21:24:24Z

@etpinard The hover is not triggered when the box is hovered on. Reading above it seemed to indicate that the box should also trigger hover, but it does not here.

etpinard · 2018-04-10T21:37:36Z

Nop. That's not what this PR intended to add. As noted in the attribute description, text is meant to annotated each sample points, not each boxes.

yniknafs · 2018-04-10T21:38:38Z

Ahh gotcha. Sounds good. So at current, there is no way to add text annotation to the box?

nmoreaud · 2020-08-04T12:43:28Z

Hello, here is an example where I add another hover information : (https://codepen.io/kiruahxh/pen/PoNYvWw)

However, I found no information about the boxplot tooltip customization/replacement (ex : How can I show a multiline text instead of having multiple tooltips? How can I translate 'median' and 'min' keywords ? How can I feed a tooltip with the statistics automatically computed by plotly.js ?)

etpinard added 8 commits October 16, 2017 18:11

lint (mostly var\n , -> var\n var blocks)

cfc8725

add lasso/select dragmode support to box traces

581f3fa

- use pts array of object to determine which box points are inside selection polygon - use index field (from Box.calc) to match each selected box point to underlying x/y input value. - dim point node opacity upon selection

flatten box test suite

e59f283

add some box hover tests

5c58049

introduce box 'hoveron'

aef61ae

- flaglist w/ 'boxes' (current + default) and 'points' - use point object x/y values (from Box.plot) to determine closest point(s) - consider jitter offset under hovermode closest, but don't under 'x' or 'y' modes

add 'text' attribute to box traces

d352fa5

- associated with each sample pt - only visible on hover (for now)

more linting in Box.plot

6a878b0

etpinard added status: reviewable feature something new labels Oct 17, 2017

etpinard added this to the v1.32.0 milestone Oct 17, 2017

etpinard commented Oct 17, 2017

View reviewed changes

alexcjohnson reviewed Oct 17, 2017

View reviewed changes

etpinard added 9 commits October 17, 2017 13:43

Merge branch 'master' into boxpoints-hover-select

0027b84

fixup box hoveron description

a1ea1e8

make 'boxes+points' the box hoveron dflt

6bfcbe0

improve boxpoints: false default logic

9ca410a

- delete 'marker' container for box trace that don't have `boxpoints: false` - do not coerce 'text' when `boxpoints: false`

add boxmode: 'overlay' hover test case

7e44c9c

aj-proof box hover logic

f228a5e

- show only 'best' boxpoint - consider opposite coordinates in compare modes to find that 'best' boxpoint - allow box AND boxpoint hover labels only in compare modes - no need for x/y w/o jitter offset values in 'pts' calc items

hide modebar select2d & lasso2d buttons for boxes w/o 'all' boxpoints

b4d8044

rm useless 'c' value in scattercarper point event data

9e69900

report coords in input-data space in pt event data

8901528

alexcjohnson reviewed Oct 19, 2017

View reviewed changes

introduce Axes.hoverLabelText

b66628e

- dry special log-off-scale hover label text routine - use it in hover.js cleanPoint - allow hoverPoints modules to set their own xLabel and yLabel value

add prefix box stats hover labels

a3ec75a

etpinard merged commit d8cff96 into master Oct 19, 2017

etpinard deleted the boxpoints-hover-select branch October 19, 2017 18:17

jackparmer mentioned this pull request Oct 19, 2017

Lasso and select-box support for traces other than scatter marker and scatter text #170

Closed

etpinard mentioned this pull request Oct 23, 2017

boxplot points hover events #192

Closed

salim-b mentioned this pull request Nov 6, 2017

add attribute boxtext to boxplots #2145

Closed

ewels mentioned this pull request Feb 26, 2018

Include process tag in the hoverover of HTML reports nextflow-io/nextflow#504

Closed

rskabelund mentioned this pull request Mar 7, 2018

Plotly.js version? plotly/plotly.py#961

Closed

Box points hover & select #2094

Box points hover & select #2094

Conversation

etpinard commented Oct 17, 2017

etpinard Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcjohnson commented Oct 19, 2017

yniknafs commented Apr 10, 2018

etpinard commented Apr 10, 2018

yniknafs commented Apr 10, 2018

etpinard commented Apr 10, 2018 • edited Loading

yniknafs commented Apr 10, 2018 via email • edited Loading

nmoreaud commented Aug 4, 2020

etpinard Oct 17, 2017 •

edited

Loading

etpinard Oct 18, 2017 •

edited

Loading

etpinard Oct 18, 2017 •

edited

Loading

etpinard Oct 17, 2017 •

edited

Loading

etpinard Oct 17, 2017 •

edited

Loading

etpinard Oct 18, 2017 •

edited

Loading

etpinard Oct 17, 2017 •

edited

Loading

etpinard commented Apr 10, 2018 •

edited

Loading

yniknafs commented Apr 10, 2018 via email •

edited

Loading