Polar polygon grids #2739

etpinard · 2018-06-15T22:26:14Z

This PR adds an attribute to polar 2.0 charts that allows users to draw polar radial grid lines and the angular axis line as polygons when the angular axis has type: 'category'. in brief, this allows users to create poorman's radar charts (i.e. radar charts with the same radial scale for all variables).

This new attribute is tentatively named polar.usepolygons. I'm not particularly happy about the name. I think we should make this a polar. attribute as it affects the look of both radial axis grid lines and the angular axis line. I'm open to suggestions.

Adding this feature turned out to a little more difficult than I thought. Quite a bit of "polygon-only" needed to be added in c3f5b1b. Getting this to work for sectors for tricky (though there's probably a simpler algo out there). We need to snap radialaxis.angle to one of the vertex angles to make sure radial axis scale remain the same as the data scale. Angular drag needed some special treatment. Here's what it looks like:

TODO:

agree on attribute name
maybe make zoombox handles follow the polygon, this would make zoombox interaction lose their rotational symmetry though - not sure what's best
maybe restrict theta values to string or "integer" to not show "out-of-scale" values
maybe make the angular drag layer a "polygon" annulus
add a few jasmine tests

cc @alexcjohnson @chriddyp

... in preparation fo polygon grids to avoid confusion

... in preperation for polygon grids where angular axis tick must be computed earlier on.

- which speeds up the category angular tick filter, and will help with polygon grids

- this adds polygons grid (and angular ax line) support to categorical angular axes - had to add 'polygon mode' to pathSector() and isPtWithinSector() - polygon vertices match the angular ticks - snaps radialaxis.angle to vertex angles - needed special care for sector and on angular drag

chriddyp · 2018-06-16T21:03:55Z

polar.usepolygons

Would something like layout.radialaxis.gridlinesshape = 'linear' | 'circular' work? Matching (somewhat) the the scatter.line.shape properties

etpinard · 2018-06-18T13:47:14Z

Would something like layout.radialaxis.gridlinesshape = 'linear' | 'circular' work?

It would work, but

it affects the look of both radial axis grid lines and the angular axis line

chriddyp · 2018-06-18T14:00:30Z

throwing some ideas out there:

radar: true
spider: true
shape: 'linear' | 'circular'

alexcjohnson · 2018-06-18T14:33:27Z

radar is ambiguous - actual radar screens have circles on them, right? And spider: false doesn’t really scream “circular” to me. So I’d prefer both shapes to be explicitly stated, like gridshape: (linear|circular).

maybe make zoombox handles follow the polygon, this would make zoombox interaction lose their rotational symmetry though

Yes. Rotational symmetry is already gone.

maybe restrict theta values to string or "integer" to not show "out-of-scale" values

This isn’t already the case for category axes?

maybe make the angular drag layer a "polygon" annulus

Would be weird if there are little gaps between the angular and radial drag areas, so probably yes.

etpinard · 2018-06-18T14:43:23Z

Yes. Rotational symmetry is already gone.

Ok. So should the cursor follow the handle, or should it just snap to the closest polygon edge?

alexcjohnson · 2018-06-18T15:39:39Z

So should the cursor follow the handle, or should it just snap to the closest polygon edge?

Mmm, it could actually be a bit weird if the handles move around everywhere the cursor goes, especially as they slip over the corners and would have to bend in arbitrary places. And there would be something nice about leaving the handles in the center of the polygon edge, where we know there's no data, only connecting lines. So yeah, I guess snap to the nearest edge center, sound reasonable?

That said, currently (circular mode) the starting handle is fixed and only the ending handle moves around with the cursor, I wonder if it wouldn't look cleaner to have the ending handle stay at the same angle as the starting handle no matter what? That's how we do it in cartesian with one axis fixedrange and I like it, makes clear that motion in the other direction is irrelevant. So I might argue for snapping the starting handle to the nearest edge center, then keeping the ending handle on the same edge no matter where the cursor goes.

Regardless (and this may already be implied but just to be clear) I think it's important that the radial value implied by any given cursor location is given by the polygon it's on, NOT its distance from the center point.

etpinard · 2018-06-19T18:11:22Z

@alexcjohnson @chriddyp

Going with gridshape: 'circular' | 'linear' in a26b56f

etpinard · 2018-06-19T20:09:40Z

I wonder if it wouldn't look cleaner to have the ending handle stay at the same angle as the starting handle no matter what?

I like it. @alexcjohnson What do you think?

etpinard · 2018-06-20T13:16:52Z

And there would be something nice about leaving the handles in the center of the polygon edge, where we know there's no data, only connecting lines. So yeah, I guess snap to the nearest edge center, sound reasonable?

Is this what you had in mind @alexcjohnson ?

etpinard · 2018-06-20T13:22:23Z

maybe restrict theta values to string or "integer" to not show "out-of-scale" values

About ⤴️ , this doesn't apply to polar subplots at the moment. Only cartesian data-ref'ed layout items support real numbers between categories. For example,

Plotly.newPlot(gd, [{
  x: ['a', 'b', 0.5],
  y: [1, 2, 3]
}, {
  type: 'scatterpolar',
   r: [1, 2, 3],
   theta: ['a', 'b', 0.5]
}], {
  xaxis: {
    type: 'category',
	domain: [0, 0.5]
  },
  polar: {
    domain: {x: [0.5, 1]},
    angularaxis: {type: 'category'}
  },
  annotations: [{
    xref: 'x', x: 1.5,
    yref: 'y', y: 2,
    showarrow: false,
    text: 'text at (1.5, 2)'
  }]
})

gives:

But we should keep this in mind when we'll add polar data-ref'ed annotations.

alexcjohnson · 2018-06-20T13:43:05Z

Is this what you had in mind?

I was imagining we'd put straight-line handles in the center of whatever polygon edge you're over - or, per our discussion yesterday on slack, whichever edge the mouse was over when you started the zoom, like this:

Also the cursor should be on the polygon edge, not at the same radius as the polygon vertices. That's what I meant above:

the radial value implied by any given cursor location is given by the polygon it's on, NOT its distance from the center point.

About ⤴️ , this doesn't apply to polar subplots at the moment. Only cartesian data-ref'ed layout items support real numbers between categories.

Got it, thanks for clarifying.

etpinard · 2018-06-20T13:47:03Z

I was imagining we'd put straight-line handles in the center of whatever polygon edge you're over

and what if the cursor is on a vertex?

alexcjohnson · 2018-06-20T14:08:03Z

and what if the cursor is on a vertex?

break the symmetry however you like - but I don't think it's worth making bent handles at vertices, in fact I'd argue it's better to use the edge centers anyway because we know we're not obscuring any data points there.

etpinard · 2018-06-20T17:32:42Z

Wow, I totally misread that comment of yours. @alexcjohnson here's my latest attempt ⬇️

... where handles are placed on the polygon under the cursor halfway between the closest vertices

etpinard · 2018-06-20T22:32:21Z

@alexcjohnson latest attempt ⤵️

alexcjohnson · 2018-06-21T14:29:28Z

Love the look and feel now, nicely done! I still see some discrepancy between mouse position and polygon edge. In this clip, I'd expect the mouse to always be exactly on an edge of the polygon, but sometimes it's inside and sometimes outside:

Also, I can't angular-drag the upper-right subplot there (the green one) - should I be able to? I don't see anything in layout.polar3 that I would expect to prevent it...

etpinard · 2018-06-21T14:49:52Z

Also, I can't angular-drag the upper-right subplot there (the green one) - should I be able to? I don't see anything in layout.polar3 that I would expect to prevent it...

See:

plotly.js/src/plots/polar/polar.js

Lines 1145 to 1149 in 40040a5

    
           // I don't what we should do in this case, skip we now 
        
           if(_this.vangles && !isFullCircle(sector)) { 
        
               dragOpts.prepFn = Lib.noop; 
        
               setCursor(d3.select(angularDrag), null); 
        
           }

- please note, the bounding box of a polygon inside a circle isn't the same as the bounding box of the circle

etpinard · 2018-06-21T21:33:38Z

I didn't expect adding almost 1000 lines for this thing, but oh well here it is ✅

alexcjohnson · 2018-06-21T22:35:14Z

test/jasmine/tests/polar_test.js

+            type: 'scatterpolar',
+            // octogons have nice angles
+            r: [1, 2, 3, 2, 3, 1, 2, 1, 2],
+            theta: ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'a']


hmm, not for this PR of course (and would be useful in circular mode as well), but this makes me realize we should have a mode in scatterpolar to join the endpoints together so you don't need to repeat the initial point when the data are meant to be periodic. trace.connectends perhaps, to mirror trace.connectgaps?

Good call. Added to -> #2255

alexcjohnson · 2018-06-21T22:42:36Z

src/plots/polar/polar.js

@@ -532,8 +532,24 @@ proto.updateAngularAxis = function(fullLayout, polarLayout) {

    Axes.doTicksSingle(gd, ax, true);

+    // angle of polygon vertices in radians (null means circles)
+    // TODO what to do when ax.period > ax._categories ??


the "angular period > _categories" subplot in your mock looks good - does that mean this TODO is done?

That mock looks good, but I'm not sure it's correct. I still don't know what the ax.period > ax._categories case can or should represent.

... and there's an item about this in #2255

alexcjohnson · 2018-06-21T23:27:04Z

src/plots/polar/polar.js

+// find intersection of 'v0' <-> 'v1' edge with a 'radial' line
+// (i.e. a line that starts from the origin at angle 'a')
+// given an (xp,yp) pair on the 'v0' <-> 'v1' line
+// (N.B. 'v0' and 'v1' are angles in radians)


whew, I think I understand the goal of this function: so you have two rays at angles v0 and v1, and given points on each ray at equal radius and a segment connecting them, first find the radius (or the segment) for which that segment intersects [xp, yp]. Then, find the point on a ray at angle a that intersects this segment. Did I get that right?

If a always matches either v0 or v1 this looks fine, but the else clause looks like it will give the wrong answer if a is anything else (since the norm of the result won't be r then).

whew, I think I understand the goal of this function: so you have two rays at angles v0 and v1, and given points on each ray at equal radius and a segment connecting them, first find the radius (or the segment) for which that segment intersects [xp, yp]. Then, find the point on a ray at angle a that intersects this segment. Did I get that right?

Yeah, you got it right except that a priori we don't know the points on each ray. We only know their angles.

If a always matches either v0 or v1 this looks fine, but the else clause looks like it will give the wrong answer if a is anything else (since the norm of the result won't be r then).

Good eye. Thanks!

When computing the the zoombox coordinates, a matches v0 so no bugs there. When computing the (clipped) polygon coordinates (when polar.sector isn't [0,360]) a is either polar.sector[0] or polar.sector[1].

At lot of little things weren't perfect for non-[0,360] polar sectors. Thanks for bringing this up. 7d405a0 fixes this.

alexcjohnson · 2018-06-21T23:27:42Z

src/plots/polar/polar.js

+        // solve g(xstar) = h(xstar)
+        var m = dsin / dcos;
+        var b = yp - m * xp;
+        var mr = Math.sin(a) / Math.cos(a);


Math.tan(a)

Actually, this isn't so 😆 , Math.tan isn't bounded. I didn't noticed anything wrong before because:

Math.tan(Math.PI / 2) // => 16331239353195370

(in Chrome 67 at least).

Oh well, 7d405a0 made things more robust.

alexcjohnson · 2018-06-22T11:37:17Z

src/plots/polar/polar.js

+    return function(index) {
+        return index < 0 ? len + index :
+            index < len ? index : index - len;
+    };


Oh, so nice. Thanks for tip. Sub'ed in -> 1ca6508

alexcjohnson · 2018-06-22T11:52:15Z

@etpinard beautiful, your trigonometry teacher would be very proud 🥇
Sounds like #2739 (comment) may contain a small bug, otherwise just minor 🐄 comments. Lets do it! 💃

alexcjohnson · 2018-06-26T19:39:29Z

Very nice. 💃 stands (or continues dancing?), lets do it!

etpinard added 6 commits June 15, 2018 17:44

rename *circle keys

0ad5b62

... in preparation fo polygon grids to avoid confusion

split radial ax autorange into own subroutine

7e16134

... in preperation for polygon grids where angular axis tick must be computed earlier on.

split out angular part of polar.isPtWithinSector into own fn

33b05eb

- which speeds up the category angular tick filter, and will help with polygon grids

add polar polygon grid mock

24276b3

fixup bgcolor for polar polygons grids

a0a62af

etpinard added feature something new status: in progress labels Jun 15, 2018

etpinard added this to the v1.39.0 milestone Jun 15, 2018

etpinard added 3 commits June 19, 2018 14:08

replace 'usepolygons' -> gridshape: 'linear'

a26b56f

draw angular draglayer as punched polygon for gridshape: 'linear'

efa0005

rotate radial axis title on angular drag

a84a5ea

etpinard added 2 commits June 20, 2018 18:11

polar polygon grids zoombox

ef4b57f

... where handles are placed on the polygon under the cursor halfway between the closest vertices

display handles at 'starting' angle

3a94ebd

DRY circular <-> linear onMove zoombox handlers

9506872

fail -> failTest

40040a5

etpinard added 3 commits June 21, 2018 13:54

add few gridshape: 'linear' tests

23476d9

clamp small deltax and deltay to avoid slopes to blow

e5ebc4b

fix x/y cursor position on drag + 🔒

b14c659

- please note, the bounding box of a polygon inside a circle isn't the same as the bounding box of the circle

alexcjohnson reviewed Jun 21, 2018

View reviewed changes

etpinard mentioned this pull request Jun 22, 2018

Polar 2.0 open items #2255

Closed

12 tasks

etpinard added status: reviewable and removed status: in progress labels Jun 22, 2018

alexcjohnson reviewed Jun 22, 2018

View reviewed changes

etpinard added 4 commits June 26, 2018 14:28

improve gridshape description

2bf3107

add eventdata test for angular drag of gridshape: 'linear' subplts

c12ff4a

fix edge-ray intersection edge cases

7d405a0

use Lib.mod to cycle indices

1ca6508

etpinard merged commit edc166f into master Jun 26, 2018

etpinard deleted the polar-polygon-grids branch June 26, 2018 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Polar polygon grids #2739

Polar polygon grids #2739

etpinard commented Jun 15, 2018 •

edited

Loading

chriddyp commented Jun 16, 2018 •

edited

Loading

etpinard commented Jun 18, 2018

chriddyp commented Jun 18, 2018

alexcjohnson commented Jun 18, 2018

etpinard commented Jun 18, 2018

alexcjohnson commented Jun 18, 2018

etpinard commented Jun 19, 2018

etpinard commented Jun 19, 2018

etpinard commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 20, 2018

etpinard commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 21, 2018

etpinard commented Jun 21, 2018

etpinard commented Jun 21, 2018

alexcjohnson Jun 21, 2018

etpinard Jun 22, 2018

alexcjohnson Jun 21, 2018

etpinard Jun 22, 2018 •

edited

Loading

etpinard Jun 22, 2018

alexcjohnson Jun 21, 2018

etpinard Jun 22, 2018

etpinard Jun 26, 2018 •

edited

Loading

alexcjohnson Jun 21, 2018

etpinard Jun 26, 2018

alexcjohnson Jun 22, 2018

etpinard Jun 26, 2018

alexcjohnson commented Jun 22, 2018

alexcjohnson commented Jun 26, 2018

Polar polygon grids #2739

Polar polygon grids #2739

Conversation

etpinard commented Jun 15, 2018 • edited Loading

chriddyp commented Jun 16, 2018 • edited Loading

etpinard commented Jun 18, 2018

chriddyp commented Jun 18, 2018

alexcjohnson commented Jun 18, 2018

etpinard commented Jun 18, 2018

alexcjohnson commented Jun 18, 2018

etpinard commented Jun 19, 2018

etpinard commented Jun 19, 2018

etpinard commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 20, 2018

etpinard commented Jun 20, 2018

etpinard commented Jun 20, 2018

alexcjohnson commented Jun 21, 2018

etpinard commented Jun 21, 2018

etpinard commented Jun 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Jun 22, 2018 • 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 Jun 26, 2018 • 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

alexcjohnson commented Jun 22, 2018

alexcjohnson commented Jun 26, 2018

etpinard commented Jun 15, 2018 •

edited

Loading

chriddyp commented Jun 16, 2018 •

edited

Loading

etpinard Jun 22, 2018 •

edited

Loading

etpinard Jun 26, 2018 •

edited

Loading