You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2018/08/19 23:04:04 UTC

[incubator-superset] branch master updated: Filter out null locations by default (#5642)

This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 4c5142d  Filter out null locations by default (#5642)
4c5142d is described below

commit 4c5142d969f6b13940acc1d19dafa0abd7cc9642
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Sun Aug 19 16:04:01 2018 -0700

    Filter out null locations by default (#5642)
    
    * Filter out null locations by default
    
    * Move exception to better place
    
    * Add unit test
    
    * Return columns in order for test and readibility
---
 superset/assets/src/explore/controls.jsx |  7 ++++
 superset/assets/src/explore/visTypes.jsx | 18 +++++-----
 superset/viz.py                          | 41 +++++++++++++++++----
 tests/viz_tests.py                       | 62 ++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+), 14 deletions(-)

diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx
index 8dd72a7..56bb950 100644
--- a/superset/assets/src/explore/controls.jsx
+++ b/superset/assets/src/explore/controls.jsx
@@ -680,6 +680,13 @@ export const controls = {
     }),
   },
 
+  filter_nulls: {
+    type: 'CheckboxControl',
+    label: t('Ignore null locations'),
+    default: true,
+    description: t('Whether to ignore locations that are null'),
+  },
+
   geojson: {
     type: 'SelectControl',
     label: t('GeoJson Column'),
diff --git a/superset/assets/src/explore/visTypes.jsx b/superset/assets/src/explore/visTypes.jsx
index 9f3e4e0..f5caae7 100644
--- a/superset/assets/src/explore/visTypes.jsx
+++ b/superset/assets/src/explore/visTypes.jsx
@@ -503,7 +503,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['spatial', 'size'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -542,7 +542,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['spatial', 'size'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -582,7 +582,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['line_column', 'line_type'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -616,7 +616,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['spatial', 'size'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -662,7 +662,8 @@ export const visTypes = {
         label: t('Query'),
         expanded: true,
         controlSetRows: [
-          ['geojson', 'row_limit'],
+          ['geojson', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -703,7 +704,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['line_column', 'line_type'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -744,7 +745,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['start_spatial', 'end_spatial'],
-          ['row_limit', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
@@ -793,7 +794,8 @@ export const visTypes = {
         label: t('Query'),
         expanded: true,
         controlSetRows: [
-          ['spatial', 'row_limit'],
+          ['spatial', null],
+          ['row_limit', 'filter_nulls'],
           ['adhoc_filters'],
         ],
       },
diff --git a/superset/viz.py b/superset/viz.py
index c060e19..1541794 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -39,7 +39,12 @@ from six.moves import cPickle as pkl, reduce
 
 from superset import app, cache, get_css_manifest_files, utils
 from superset.exceptions import NullValueException, SpatialException
-from superset.utils import DTTM_ALIAS, JS_MAX_INTEGER, merge_extra_filters
+from superset.utils import (
+    DTTM_ALIAS,
+    JS_MAX_INTEGER,
+    merge_extra_filters,
+    to_adhoc,
+)
 
 
 config = app.config
@@ -2070,17 +2075,19 @@ class BaseDeckGLViz(BaseViz):
         return [self.metric] if self.metric else []
 
     def process_spatial_query_obj(self, key, group_by):
+        group_by.extend(self.get_spatial_columns(key))
+
+    def get_spatial_columns(self, key):
         spatial = self.form_data.get(key)
         if spatial is None:
             raise ValueError(_('Bad spatial key'))
 
         if spatial.get('type') == 'latlong':
-            group_by += [spatial.get('lonCol')]
-            group_by += [spatial.get('latCol')]
+            return [spatial.get('lonCol'), spatial.get('latCol')]
         elif spatial.get('type') == 'delimited':
-            group_by += [spatial.get('lonlatCol')]
+            return [spatial.get('lonlatCol')]
         elif spatial.get('type') == 'geohash':
-            group_by += [spatial.get('geohashCol')]
+            return [spatial.get('geohashCol')]
 
     @staticmethod
     def parse_coordinates(s):
@@ -2123,9 +2130,31 @@ class BaseDeckGLViz(BaseViz):
                                        please consider filtering those out'))
         return df
 
+    def add_null_filters(self):
+        spatial_columns = set()
+        for key in self.spatial_control_keys:
+            for column in self.get_spatial_columns(key):
+                spatial_columns.add(column)
+
+        if self.form_data.get('adhoc_filters') is None:
+            self.form_data['adhoc_filters'] = []
+
+        for column in sorted(spatial_columns):
+            filter_ = to_adhoc({
+                'col': column,
+                'op': 'IS NOT NULL',
+                'val': '',
+            })
+            self.form_data['adhoc_filters'].append(filter_)
+
     def query_obj(self):
-        d = super(BaseDeckGLViz, self).query_obj()
         fd = self.form_data
+
+        # add NULL filters
+        if fd.get('filter_nulls'):
+            self.add_null_filters()
+
+        d = super(BaseDeckGLViz, self).query_obj()
         gb = []
 
         for key in self.spatial_control_keys:
diff --git a/tests/viz_tests.py b/tests/viz_tests.py
index 61539b6..3cc2f86 100644
--- a/tests/viz_tests.py
+++ b/tests/viz_tests.py
@@ -6,6 +6,7 @@ from __future__ import unicode_literals
 
 from datetime import datetime
 import unittest
+import uuid
 
 from mock import Mock, patch
 import pandas as pd
@@ -950,6 +951,67 @@ class BaseDeckGLVizTestCase(unittest.TestCase):
         with self.assertRaises(SpatialException):
             test_viz_deckgl.parse_coordinates('fldkjsalkj,fdlaskjfjadlksj')
 
+    @patch('superset.utils.uuid.uuid4')
+    def test_filter_nulls(self, mock_uuid4):
+        mock_uuid4.return_value = uuid.UUID('12345678123456781234567812345678')
+        test_form_data = {
+            'latlong_key': {
+                'type': 'latlong',
+                'lonCol': 'lon',
+                'latCol': 'lat',
+            },
+            'delimited_key': {
+                'type': 'delimited',
+                'lonlatCol': 'lonlat',
+            },
+            'geohash_key': {
+                'type': 'geohash',
+                'geohashCol': 'geo',
+            },
+        }
+
+        datasource = {'type': 'table'}
+        expected_results = {
+            'latlong_key': [{
+                'clause': 'WHERE',
+                'expressionType': 'SIMPLE',
+                'filterOptionName': '12345678-1234-5678-1234-567812345678',
+                'comparator': '',
+                'operator': 'IS NOT NULL',
+                'subject': 'lat',
+            }, {
+                'clause': 'WHERE',
+                'expressionType': 'SIMPLE',
+                'filterOptionName': '12345678-1234-5678-1234-567812345678',
+                'comparator': '',
+                'operator': 'IS NOT NULL',
+                'subject': 'lon',
+            }],
+            'delimited_key': [{
+                'clause': 'WHERE',
+                'expressionType': 'SIMPLE',
+                'filterOptionName': '12345678-1234-5678-1234-567812345678',
+                'comparator': '',
+                'operator': 'IS NOT NULL',
+                'subject': 'lonlat',
+            }],
+            'geohash_key': [{
+                'clause': 'WHERE',
+                'expressionType': 'SIMPLE',
+                'filterOptionName': '12345678-1234-5678-1234-567812345678',
+                'comparator': '',
+                'operator': 'IS NOT NULL',
+                'subject': 'geo',
+            }],
+        }
+        for mock_key in ['latlong_key', 'delimited_key', 'geohash_key']:
+            test_viz_deckgl = viz.BaseDeckGLViz(
+                datasource, test_form_data.copy())
+            test_viz_deckgl.spatial_control_keys = [mock_key]
+            test_viz_deckgl.add_null_filters()
+            adhoc_filters = test_viz_deckgl.form_data['adhoc_filters']
+            assert expected_results.get(mock_key) == adhoc_filters
+
 
 class TimeSeriesVizTestCase(unittest.TestCase):