From 8d8f1c1ad857530aa3d4d1dcded4fd5c4205029c Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 16 Jul 2018 13:42:07 -0700 Subject: [PATCH] [bugfix] make MetricsControl work with DECK visualizations (#5376) * [bugfix] make MetricsControl work with DECK visualizations * Add unit tests (cherry picked from commit 709f056445549c5fcb5fe2020b7539eb29dcf29c) (cherry picked from commit 23262ae988310ea406ff374a2ece86949be1c228) --- .../controls/FixedOrMetricControl.jsx | 4 +- superset/assets/src/explore/visTypes.js | 1 + superset/assets/src/logger.js | 2 + superset/exceptions.py | 4 ++ superset/viz.py | 50 +++++++++++++------ tests/viz_tests.py | 28 +++++++++++ 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx b/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx index df4eec0cb9dcc..d330b2fb2c993 100644 --- a/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx +++ b/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx @@ -63,7 +63,7 @@ export default class FixedOrMetricControl extends React.Component { renderPopover() { const value = this.props.value || this.props.default; const type = value.type || controlTypes.fixed; - const metrics = this.props.datasource ? this.props.datasource.metrics : null; + const metricsCombo = this.props.datasource ? this.props.datasource.metrics_combo : null; return (
@@ -87,7 +87,7 @@ export default class FixedOrMetricControl extends React.Component { { this.setType(controlTypes.metric); }} onChange={this.setMetric} value={this.state.metricValue} diff --git a/superset/assets/src/explore/visTypes.js b/superset/assets/src/explore/visTypes.js index cb6bf90c84dc3..43188c411f099 100644 --- a/superset/assets/src/explore/visTypes.js +++ b/superset/assets/src/explore/visTypes.js @@ -632,6 +632,7 @@ export const visTypes = { }, { label: t('Grid'), + expanded: true, controlSetRows: [ ['grid_size', 'color_picker'], ], diff --git a/superset/assets/src/logger.js b/superset/assets/src/logger.js index ea8e0fbf9d8f4..358cbf7e9a4b7 100644 --- a/superset/assets/src/logger.js +++ b/superset/assets/src/logger.js @@ -11,6 +11,7 @@ export const Logger = { if (!addEventHandlers[eventName]) { addEventHandlers[eventName] = log.addEvent.bind(log); } else { + // eslint-disable-next-line no-console console.warn(`Duplicate event handler for event '${eventName}'`); } }); @@ -20,6 +21,7 @@ export const Logger = { if (addEventHandlers[eventName]) { addEventHandlers[eventName](eventName, eventBody, sendNow); } else { + // eslint-disable-next-line no-console console.warn(`No event handler for event '${eventName}'`); } }, diff --git a/superset/exceptions.py b/superset/exceptions.py index 8059439452a44..610acea79aa12 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -32,3 +32,7 @@ class NullValueException(SupersetException): class SupersetTemplateException(SupersetException): pass + + +class SpatialException(SupersetException): + pass diff --git a/superset/viz.py b/superset/viz.py index e9740fe5a2e6d..cad28cbdca4c1 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -38,7 +38,7 @@ from six.moves import cPickle as pkl, reduce from superset import app, cache, get_manifest_file, utils -from superset.exceptions import NullValueException +from superset.exceptions import NullValueException, SpatialException from superset.utils import DTTM_ALIAS, JS_MAX_INTEGER, merge_extra_filters @@ -2083,6 +2083,17 @@ def process_spatial_query_obj(self, key, group_by): elif spatial.get('type') == 'geohash': group_by += [spatial.get('geohashCol')] + @staticmethod + def parse_coordinates(s): + if not s: + return None + try: + p = Point(s) + except Exception: + raise SpatialException( + _('Invalid spatial point encountered: %s' % s)) + return (p.latitude, p.longitude) + def process_spatial_data_obj(self, key, df): spatial = self.form_data.get(key) if spatial is None: @@ -2093,19 +2104,15 @@ def process_spatial_data_obj(self, key, df): pd.to_numeric(df[spatial.get('latCol')], errors='coerce'), )) elif spatial.get('type') == 'delimited': - - def tupleify(s): - p = Point(s) - return (p.latitude, p.longitude) - - df[key] = df[spatial.get('lonlatCol')].apply(tupleify) + lon_lat_col = spatial.get('lonlatCol') + df[key] = df[lon_lat_col].apply(self.parse_coordinates) if spatial.get('reverseCheckbox'): df[key] = [ tuple(reversed(o)) if isinstance(o, (list, tuple)) else (0, 0) for o in df[key] ] - del df[spatial.get('lonlatCol')] + del df[lon_lat_col] elif spatial.get('type') == 'geohash': latlong = df[spatial.get('geohashCol')].map(geohash.decode) df[key] = list(zip(latlong.apply(lambda x: x[0]), @@ -2115,7 +2122,6 @@ def tupleify(s): if df.get(key) is None: raise NullValueException(_('Encountered invalid NULL spatial entry, \ please consider filtering those out')) - return df def query_obj(self): @@ -2149,6 +2155,8 @@ def get_js_columns(self, d): def get_data(self, df): if df is None: return None + + # Processing spatial info for key in self.spatial_control_keys: df = self.process_spatial_data_obj(key, df) @@ -2195,8 +2203,8 @@ def get_metrics(self): def get_properties(self, d): return { - 'metric': d.get(self.metric), - 'radius': self.fixed_value if self.fixed_value else d.get(self.metric), + 'metric': d.get(self.metric_label), + 'radius': self.fixed_value if self.fixed_value else d.get(self.metric_label), 'cat_color': d.get(self.dim) if self.dim else None, 'position': d.get('spatial'), DTTM_ALIAS: d.get(DTTM_ALIAS), @@ -2204,6 +2212,8 @@ def get_properties(self, d): def get_data(self, df): fd = self.form_data + self.metric_label = \ + self.get_metric_label(self.metric) if self.metric else None self.point_radius_fixed = fd.get('point_radius_fixed') self.fixed_value = None self.dim = self.form_data.get('dimension') @@ -2229,10 +2239,14 @@ def query_obj(self): def get_properties(self, d): return { 'position': d.get('spatial'), - 'weight': d.get(self.metric) or 1, + 'weight': d.get(self.metric_label) or 1, '__timestamp': d.get(DTTM_ALIAS) or d.get('__time'), } + def get_data(self, df): + self.metric_label = self.get_metric_label(self.metric) + return super(DeckScreengrid, self).get_data(df) + class DeckGrid(BaseDeckGLViz): @@ -2245,9 +2259,13 @@ class DeckGrid(BaseDeckGLViz): def get_properties(self, d): return { 'position': d.get('spatial'), - 'weight': d.get(self.metric) or 1, + 'weight': d.get(self.metric_label) or 1, } + def get_data(self, df): + self.metric_label = self.get_metric_label(self.metric) + return super(DeckGrid, self).get_data(df) + class DeckPathViz(BaseDeckGLViz): @@ -2301,9 +2319,13 @@ class DeckHex(BaseDeckGLViz): def get_properties(self, d): return { 'position': d.get('spatial'), - 'weight': d.get(self.metric) or 1, + 'weight': d.get(self.metric_label) or 1, } + def get_data(self, df): + self.metric_label = self.get_metric_label(self.metric) + return super(DeckHex, self).get_data(df) + class DeckGeoJson(BaseDeckGLViz): diff --git a/tests/viz_tests.py b/tests/viz_tests.py index 1bc6e62a97448..4c63f0cfa3346 100644 --- a/tests/viz_tests.py +++ b/tests/viz_tests.py @@ -10,6 +10,8 @@ from mock import Mock, patch import pandas as pd +from superset import app +from superset.exceptions import SpatialException from superset.utils import DTTM_ALIAS import superset.viz as viz from .utils import load_fixture @@ -940,3 +942,29 @@ def test_geojson_query_obj(self): assert results['metrics'] == [] assert results['groupby'] == [] assert results['columns'] == ['test_col'] + + def test_parse_coordinates(self): + form_data = load_fixture('deck_path_form_data.json') + datasource = {'type': 'table'} + viz_instance = viz.BaseDeckGLViz(datasource, form_data) + + coord = viz_instance.parse_coordinates('1.23, 3.21') + self.assertEquals(coord, (1.23, 3.21)) + + coord = viz_instance.parse_coordinates('1.23 3.21') + self.assertEquals(coord, (1.23, 3.21)) + + self.assertEquals(viz_instance.parse_coordinates(None), None) + + self.assertEquals(viz_instance.parse_coordinates(''), None) + + def test_parse_coordinates_raises(self): + form_data = load_fixture('deck_path_form_data.json') + datasource = {'type': 'table'} + test_viz_deckgl = viz.BaseDeckGLViz(datasource, form_data) + + with self.assertRaises(SpatialException): + test_viz_deckgl.parse_coordinates('NULL') + + with self.assertRaises(SpatialException): + test_viz_deckgl.parse_coordinates('fldkjsalkj,fdlaskjfjadlksj')