Make polar be more graceful on non sense inputs #3021
Conversation
- this didn't lead to any bugs, as
var a = ['x' + 'y']
var b = {}
b[a] = 1
// => {'xy': 1}
| y2: 0, | ||
| transform: trans | ||
| }) | ||
| updateElement( |
There was a problem hiding this comment.
Recall that updateElement is
plotly.js/src/plots/polar/polar.js
Lines 1347 to 1355 in 7f0fc26
| }); | ||
|
|
||
| it('@gl should fire *plotly_webglcontextlost* when on webgl context lost', function(done) { | ||
| it('@noCI @gl should fire *plotly_webglcontextlost* when on webgl context lost', function(done) { |
There was a problem hiding this comment.
4 successes in 4 test-jasmine2 runs with this @noCI
| }]) | ||
| .then(function() { _assert('base', true); }) | ||
| .then(function() { return Plotly.relayout(gd, 'polar.hole', 1); }) | ||
| .then(function() { _assert('hole=1', false); }) |
There was a problem hiding this comment.
So it doesn't fail... does it work? ie draws all data on the rim regardless of radial value (within the radial range)? Do you want to include this in a mock?
There was a problem hiding this comment.
Here's hole=1 subplot:
where angular drag still drag still works.
ie draws all data on the rim regardless of radial value (within the radial range)?
it shows no data points.
Do you want to include this in a mock?
done in cb5b336
There was a problem hiding this comment.
it shows no data points.
This comment is nonblocking (perhaps add to #2255 if you don't want to address it here):
With cliponaxis: false (which is the default anyway for polar) I would have expected it to show the data - and it currently doesn't, even if you explicitly specify a range. The argument for this behavior is to have hole: 1 match hole: 0.9999 as closely as possible. Perhaps not useful, though I guess I could see someone wanting to just show where their data is on an angular scale without involving a radial scale at all. That said they can always just use hole: 0.9999 as a workaround, so it's not really any missing functionality... but it feels hacky.
There was a problem hiding this comment.
The argument for this behavior is to have
hole: 1matchhole: 0.9999as closely as possible.
Ah I see. Good point.
Perhaps not useful, though I guess I could see someone wanting to just show where their data is on an angular scale without involving a radial scale at all
I'll wait until someone complains about it (maybe @nicolaskruchten ?) before addressing this. Now in -> #2255 (comment)
|
Very nice bunch of cleanup @etpinard! 💃 |
This PR fixes two issue-less bugs discovered by @dmt0:
polar.hole=1case https://codepen.io/dmt0/pen/pOVdKxpolar.sectorthat spans more than 360 degrees https://codepen.io/dmt0/pen/LJmOQVThis PR also:
plotly_webglcontextlosttest ingl2d_plot_interact_test.jsas@noCI.@flakyThe last two items should help with the (several) intermittent test failures we've been seeing lately.
cc @alexcjohnson @antoinerg