You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/11/17 01:06:23 UTC

[GitHub] mistercrunch closed pull request #6399: Fix adhoc metrics in Polygon

mistercrunch closed pull request #6399: Fix adhoc metrics in Polygon
URL: https://github.com/apache/incubator-superset/pull/6399
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/spec/javascripts/visualizations/deckgl/utils_spec.js b/superset/assets/spec/javascripts/visualizations/deckgl/utils_spec.js
index 16ed41edfa..be4a44b62b 100644
--- a/superset/assets/spec/javascripts/visualizations/deckgl/utils_spec.js
+++ b/superset/assets/spec/javascripts/visualizations/deckgl/utils_spec.js
@@ -4,6 +4,8 @@ import {
   getBuckets,
 } from '../../../../src/visualizations/deckgl/utils';
 
+const metricAccessor = d => d.count;
+
 describe('getBreakPoints', () => {
   it('is a function', () => {
     expect(typeof getBreakPoints).toBe('function');
@@ -11,7 +13,7 @@ describe('getBreakPoints', () => {
 
   it('returns sorted break points', () => {
     const fd = { break_points: ['0', '10', '100', '50', '1000'] };
-    const result = getBreakPoints(fd, []);
+    const result = getBreakPoints(fd, [], metricAccessor);
     const expected = ['0', '10', '50', '100', '1000'];
     expect(result).toEqual(expected);
   });
@@ -19,7 +21,7 @@ describe('getBreakPoints', () => {
   it('returns evenly distributed break points when no break points are specified', () => {
     const fd = { metric: 'count' };
     const features = [0, 1, 2, 10].map(count => ({ count }));
-    const result = getBreakPoints(fd, features);
+    const result = getBreakPoints(fd, features, metricAccessor);
     const expected = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10'];
     expect(result).toEqual(expected);
   });
@@ -27,7 +29,7 @@ describe('getBreakPoints', () => {
   it('formats number with proper precision', () => {
     const fd = { metric: 'count', num_buckets: 2 };
     const features = [0, 1 / 3, 2 / 3, 1].map(count => ({ count }));
-    const result = getBreakPoints(fd, features);
+    const result = getBreakPoints(fd, features, metricAccessor);
     const expected = ['0.0', '0.5', '1.0'];
     expect(result).toEqual(expected);
   });
@@ -35,7 +37,7 @@ describe('getBreakPoints', () => {
   it('works with a zero range', () => {
     const fd = { metric: 'count', num_buckets: 1 };
     const features = [1, 1, 1].map(count => ({ count }));
-    const result = getBreakPoints(fd, features);
+    const result = getBreakPoints(fd, features, metricAccessor);
     const expected = ['1', '1'];
     expect(result).toEqual(expected);
   });
@@ -53,7 +55,7 @@ describe('getBreakPointColorScaler', () => {
       opacity: 100,
     };
     const features = [10, 20, 30].map(count => ({ count }));
-    const scaler = getBreakPointColorScaler(fd, features);
+    const scaler = getBreakPointColorScaler(fd, features, metricAccessor);
     expect(scaler({ count: 10 })).toEqual([0, 0, 0, 255]);
     expect(scaler({ count: 15 })).toEqual([64, 64, 64, 255]);
     expect(scaler({ count: 30 })).toEqual([255, 255, 255, 255]);
@@ -67,7 +69,7 @@ describe('getBreakPointColorScaler', () => {
       opacity: 100,
     };
     const features = [];
-    const scaler = getBreakPointColorScaler(fd, features);
+    const scaler = getBreakPointColorScaler(fd, features, metricAccessor);
     expect(scaler({ count: 0 })).toEqual([0, 0, 0, 255]);
     expect(scaler({ count: 0.5 })).toEqual([0, 0, 0, 255]);
     expect(scaler({ count: 1 })).toEqual([255, 255, 255, 255]);
@@ -82,7 +84,7 @@ describe('getBreakPointColorScaler', () => {
       opacity: 100,
     };
     const features = [];
-    const scaler = getBreakPointColorScaler(fd, features);
+    const scaler = getBreakPointColorScaler(fd, features, metricAccessor);
     expect(scaler({ count: -1 })).toEqual([0, 0, 0, 0]);
     expect(scaler({ count: 11 })).toEqual([255, 255, 255, 0]);
   });
@@ -101,7 +103,7 @@ describe('getBuckets', () => {
       opacity: 100,
     };
     const features = [];
-    const result = getBuckets(fd, features);
+    const result = getBuckets(fd, features, metricAccessor);
     const expected = {
       '0 - 1': { color: [0, 0, 0, 255], enabled: true },
       '1 - 10': { color: [255, 255, 255, 255], enabled: true },
diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index 67ae006a07..2a35fbe1af 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -126,8 +126,7 @@ class Chart extends React.PureComponent {
 
   renderTooltip() {
     const { tooltip } = this.state;
-
-    if (tooltip) {
+    if (tooltip && tooltip.content) {
       return (
         <Tooltip
           className="chart-tooltip"
@@ -173,32 +172,31 @@ class Chart extends React.PureComponent {
     this.renderStartTime = Logger.getTimestamp();
 
     return (
-      <div
-        className={`chart-container ${isLoading ? 'is-loading' : ''}`}
-        style={containerStyles}
-      >
-        {this.renderTooltip()}
-
-        {['loading', 'success'].indexOf(chartStatus) >= 0 && <Loading size={50} />}
-
-        {chartAlert && (
-          <StackTraceMessage
-            message={chartAlert}
-            link={queryResponse ? queryResponse.link : null}
-            stackTrace={chartStackTrace}
-          />
-        )}
-
-        {!isLoading && !chartAlert && isFaded && (
-          <RefreshChartOverlay
-            width={width}
-            height={height}
-            onQuery={onQuery}
-            onDismiss={onDismissRefreshOverlay}
-          />
-        )}
+      <ErrorBoundary onError={this.handleRenderFailure} showMessage={false}>
+        <div
+          className={`chart-container ${isLoading ? 'is-loading' : ''}`}
+          style={containerStyles}
+        >
+          {this.renderTooltip()}
 
-        <ErrorBoundary onError={this.handleRenderFailure} showMessage={false}>
+          {['loading', 'success'].indexOf(chartStatus) >= 0 && <Loading size={50} />}
+
+          {chartAlert && (
+            <StackTraceMessage
+              message={chartAlert}
+              link={queryResponse ? queryResponse.link : null}
+              stackTrace={chartStackTrace}
+            />
+          )}
+
+          {!isLoading && !chartAlert && isFaded && (
+            <RefreshChartOverlay
+              width={width}
+              height={height}
+              onQuery={onQuery}
+              onDismiss={onDismissRefreshOverlay}
+            />
+          )}
           <SuperChart
             className={`slice_container ${snakeCase(vizType)} ${isFaded ? ' faded' : ''}`}
             chartType={vizType}
@@ -207,8 +205,8 @@ class Chart extends React.PureComponent {
             onRenderFailure={this.handleRenderFailure}
             skipRendering={skipChartRendering}
           />
-        </ErrorBoundary>
-      </div>
+        </div>
+      </ErrorBoundary>
     );
   }
 }
diff --git a/superset/assets/src/visualizations/deckgl/layers/Polygon/Polygon.jsx b/superset/assets/src/visualizations/deckgl/layers/Polygon/Polygon.jsx
index a245023d81..21de46ed69 100644
--- a/superset/assets/src/visualizations/deckgl/layers/Polygon/Polygon.jsx
+++ b/superset/assets/src/visualizations/deckgl/layers/Polygon/Polygon.jsx
@@ -48,10 +48,12 @@ export function getLayer(formData, payload, setTooltip, selected, onSelect, filt
     data = jsFnMutator(data);
   }
 
+  const metricLabel = fd.metric ? fd.metric.label || fd.metric : null;
+  const accessor = d => d[metricLabel];
   // base color for the polygons
   const baseColorScaler = fd.metric === null
     ? () => [fc.r, fc.g, fc.b, 255 * fc.a]
-    : getBreakPointColorScaler(fd, data);
+    : getBreakPointColorScaler(fd, data, accessor);
 
   // when polygons are selected, reduce the opacity of non-selected polygons
   const colorScaler = (d) => {
@@ -61,7 +63,6 @@ export function getLayer(formData, payload, setTooltip, selected, onSelect, filt
     }
     return baseColor;
   };
-
   return new PolygonLayer({
     id: `path-layer-${fd.slice_id}`,
     data,
@@ -211,7 +212,12 @@ class DeckGLPolygon extends React.Component {
   render() {
     const { payload, formData, setControlValue } = this.props;
     const { start, end, getStep, values, disabled, viewport } = this.state;
-    const buckets = getBuckets(formData, payload.data.features);
+
+    const fd = formData;
+    const metricLabel = fd.metric ? fd.metric.label || fd.metric : null;
+    const accessor = d => d[metricLabel];
+
+    const buckets = getBuckets(formData, payload.data.features, accessor);
     return (
       <div style={{ position: 'relative' }}>
         <AnimatableDeckGLContainer
diff --git a/superset/assets/src/visualizations/deckgl/layers/common.jsx b/superset/assets/src/visualizations/deckgl/layers/common.jsx
index 36b05cefa9..02ec70f39d 100644
--- a/superset/assets/src/visualizations/deckgl/layers/common.jsx
+++ b/superset/assets/src/visualizations/deckgl/layers/common.jsx
@@ -38,12 +38,13 @@ export function commonLayerProps(formData, setTooltip, onSelect) {
   let tooltipContentGenerator;
   if (fd.js_tooltip) {
     tooltipContentGenerator = sandboxedEval(fd.js_tooltip);
-  } else if (fd.line_column && fd.line_type === 'geohash') {
+  } else if (fd.line_column && fd.metric && ['geohash', 'zipcode'].indexOf(fd.line_type) >= 0) {
+    const metricLabel = fd.metric.label || fd.metric;
     tooltipContentGenerator = o => (
       <div>
         <div>{fd.line_column}: <strong>{o.object[fd.line_column]}</strong></div>
         {fd.metric &&
-          <div>{fd.metric}: <strong>{o.object[fd.metric]}</strong></div>}
+          <div>{metricLabel}: <strong>{o.object[metricLabel]}</strong></div>}
       </div>);
   }
   if (tooltipContentGenerator) {
diff --git a/superset/assets/src/visualizations/deckgl/utils.js b/superset/assets/src/visualizations/deckgl/utils.js
index 533ec58487..b692f6f69b 100644
--- a/superset/assets/src/visualizations/deckgl/utils.js
+++ b/superset/assets/src/visualizations/deckgl/utils.js
@@ -3,20 +3,19 @@ import { scaleThreshold } from 'd3-scale';
 import { getSequentialSchemeRegistry, SequentialScheme } from '@superset-ui/color';
 import { hexToRGB } from '../../modules/colors';
 
+const DEFAULT_NUM_BUCKETS = 10;
+
 export function getBreakPoints({
     break_points: formDataBreakPoints,
     num_buckets: formDataNumBuckets,
-    metric,
-  }, features) {
+  }, features, accessor) {
   if (!features) {
     return [];
   }
   if (formDataBreakPoints === undefined || formDataBreakPoints.length === 0) {
     // compute evenly distributed break points based on number of buckets
-    const numBuckets = formDataNumBuckets
-      ? parseInt(formDataNumBuckets, 10)
-      : 10;
-    const [minValue, maxValue] = extent(features, d => d[metric]);
+    const numBuckets = formDataNumBuckets ? parseInt(formDataNumBuckets, 10) : DEFAULT_NUM_BUCKETS;
+    const [minValue, maxValue] = extent(features, accessor);
     const delta = (maxValue - minValue) / numBuckets;
     const precision = delta === 0
       ? 0
@@ -32,15 +31,13 @@ export function getBreakPointColorScaler({
     break_points: formDataBreakPoints,
     num_buckets: formDataNumBuckets,
     linear_color_scheme: linearColorScheme,
-    metric,
     opacity,
-  }, features) {
+  }, features, accessor) {
   const breakPoints = formDataBreakPoints || formDataNumBuckets
     ? getBreakPoints({
       break_points: formDataBreakPoints,
       num_buckets: formDataNumBuckets,
-      metric,
-    }, features)
+    }, features, accessor)
     : null;
   const colorScheme = Array.isArray(linearColorScheme)
     ? new SequentialScheme({
@@ -69,13 +66,14 @@ export function getBreakPointColorScaler({
     maskPoint = value => value > breakPoints[n] || value < breakPoints[0];
   } else {
     // interpolate colors linearly
-    scaler = colorScheme.createLinearScale(extent(features, d => d[metric]));
+    scaler = colorScheme.createLinearScale(extent(features, accessor));
     maskPoint = () => false;
   }
 
   return (d) => {
-    const c = hexToRGB(scaler(d[metric]));
-    if (maskPoint(d[metric])) {
+    const v = accessor(d);
+    const c = hexToRGB(scaler(v));
+    if (maskPoint(v)) {
       c[3] = 0;
     } else {
       c[3] = (opacity / 100.0) * 255;
@@ -84,15 +82,15 @@ export function getBreakPointColorScaler({
   };
 }
 
-export function getBuckets(fd, features) {
-  const breakPoints = getBreakPoints(fd, features, true);
-  const colorScaler = getBreakPointColorScaler(fd, features);
+export function getBuckets(fd, features, accessor) {
+  const breakPoints = getBreakPoints(fd, features, accessor);
+  const colorScaler = getBreakPointColorScaler(fd, features, accessor);
   const buckets = {};
   breakPoints.slice(1).forEach((value, i) => {
     const range = breakPoints[i] + ' - ' + breakPoints[i + 1];
     const mid = 0.5 * (parseInt(breakPoints[i], 10) + parseInt(breakPoints[i + 1], 10));
     buckets[range] = {
-      color: colorScaler({ [fd.metric]: mid }),
+      color: colorScaler({ [fd.metric.label || fd.metric]: mid }),
       enabled: true,
     };
   });


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org