misc improvements after AJ's feedback

etpinard · etpinard · commit 884afdfdbacb · 2018-03-08T23:27:49.000-05:00
- dive into 2d arrays to estimate their lengths
- bump box, violin, histogram and histogram2d max pt threshold to 1e6,
  and add special case for box and violin with sample points displayed
- use budget so that multiple traces close to threshold won't hang exporter
- make traces w/o array don't mess up count
diff --git a/src/component/plotly-graph/parse.js b/src/component/plotly-graph/parse.js
@@ -120,30 +120,42 @@ function willFigureHang (result) {
     return true
   }
 
-  const numberOfPtsPerTraceType = {}
+  let maxPtBudget = 0
 
   for (let i = 0; i < data.length; i++) {
     const trace = data[i] || {}
     const type = trace.type || 'scatter'
+    const len = estimateDataLength(trace)
 
-    if (!(type in numberOfPtsPerTraceType)) {
-      numberOfPtsPerTraceType[type] = 0
-    }
+    // cap the number of points using a budget
+    maxPtBudget += len / maxPtsPerTraceType(type)
+    if (maxPtBudget > 1) return true
 
-    numberOfPtsPerTraceType[type] += estimateDataLength(trace)
+    // other special cases
 
-    // cap the number of point per trace type
-    if (numberOfPtsPerTraceType[type] > maxPtsPerTraceType(type)) {
+    // box with boxpoints: 'all'
+    if (
+      type === 'box' &&
+      trace.boxpoints === 'all' &&
+      len > 5e4
+    ) {
       return true
     }
 
-    // other special cases
+    // violin with points: 'all'
+    if (
+      type === 'violin' &&
+      trace.points === 'all' &&
+      len > 5e4
+    ) {
+      return true
+    }
 
     // mesh3d will `alphahull` and 1000+ pts
     if (
       type === 'mesh3d' &&
       'alphahull' in trace && Number(trace.alphahull) >= 0 &&
-      findMaxArrayLength(trace) > 1000
+      len > 1000
     ) {
       return true
     }
@@ -155,11 +167,23 @@ function willFigureHang (result) {
 // can be (much) smaller than the true number of points plotted
 // when it does not match the length of the other coordinate arrays.
 function findMaxArrayLength (cont) {
-  const lengths = Object.keys(cont)
+  const arrays = Object.keys(cont)
     .filter(k => Array.isArray(cont[k]))
-    .map(k => cont[k].length)
+    .map(k => cont[k])
 
-  return Math.max(...lengths)
+  const lengths = arrays.map(arr => {
+    const innerArrays = arr.filter(Array.isArray)
+
+    if (innerArrays.length) {
+      return innerArrays
+        .map(a => a.length)
+        .reduce((a, v) => a + v)
+    } else {
+      return arr.length
+    }
+  })
+
+  return arrays.length ? Math.max(...lengths) : 0
 }
 
 function estimateDataLength (trace) {
@@ -200,7 +224,7 @@ function maxPtsPerTraceType (type) {
     case 'histogram2dcontour':
     case 'box':
     case 'violin':
-      return 1e5
+      return 1e6
 
     default:
       return 5e4
diff --git a/test/unit/plotly-graph_test.js b/test/unit/plotly-graph_test.js
@@ -376,6 +376,25 @@ tap.test('parse:', t => {
       })
     })
 
+    t.test('failing heatmap case', t => {
+      var z = [
+        new Array(5e5),
+        new Array(5e5),
+        new Array(5e5)
+      ]
+
+      fn({
+        data: [{
+          type: 'heatmap',
+          z: z
+        }]
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
     t.test('failing case from too many traces', t => {
       var data = new Array(3e3)
 
@@ -388,6 +407,34 @@ tap.test('parse:', t => {
       })
     })
 
+    t.test('failing edge case (box with boxpoints all)', t => {
+      fn({
+        data: [{
+          type: 'box',
+          x: new Array(1e5),
+          boxpoints: 'all'
+        }]
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
+    t.test('failing edge case (violin with points all)', t => {
+      fn({
+        data: [{
+          type: 'violin',
+          x: new Array(1e5),
+          points: 'all'
+        }]
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
     t.test('failing edge case (mesh3d and alphahull)', t => {
       var data = new Array(2e3)
 
@@ -404,6 +451,50 @@ tap.test('parse:', t => {
       })
     })
 
+    t.test('failing case from too many traces', t => {
+      var data = new Array(3e3)
+
+      fn({
+        data: data
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
+    t.test('failing edge case (to test budget)', t => {
+      fn({
+        data: [{
+          type: 'scatter',
+          x: new Array(4e4) // below 5e4 threshold
+        }, {
+          type: 'heatmap',
+          z: [
+            new Array(5e5), // below 5e4 threshold
+            new Array(4e5)
+          ]
+        }]
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
+    t.test('failing case (with no arrays in starting trace)', t => {
+      fn({
+        data: [{}, {}, {
+          type: 'scatter',
+          x: new Array(1e6)
+        }]
+      }, {}, (errorCode, result) => {
+        t.equal(errorCode, 400, 'code')
+        t.type(result.msg, 'string', 'msg type')
+        t.end()
+      })
+    })
+
     t.end()
   })
 
