From 01aa3a2865ab6b8c4835604d8e78f8e8fac7f478 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Thu, 12 Apr 2018 22:06:42 +0000
Subject: [PATCH] Addressing more comments

---
 superset/connectors/base/models.py  | 13 +++++++------
 superset/connectors/druid/models.py |  3 ++-
 superset/connectors/sqla/models.py  |  3 ++-
 superset/viz.py                     |  2 --
 tests/druid_func_tests.py           |  8 ++++----
 5 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index 87c0fc2c63ed7..8e4a2a22459d4 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -192,19 +192,20 @@ def filter_values_handler(
         def handle_single_value(v):
             # backward compatibility with previous <select> components
             if isinstance(v, basestring):
-                v = v.strip().strip("'").strip('"')
+                v = v.strip('\t\n \'"')
                 if target_column_is_numeric:
                     # For backwards compatibility and edge cases
                     # where a column data type might have changed
                     v = utils.string_to_num(v)
-            if v == '<NULL>':
-                return None
-            elif v == '<empty string>':
-                return ''
+                if v == '<NULL>':
+                    return None
+                elif v == '<empty string>':
+                    return ''
             return v
         if isinstance(values, (list, tuple)):
             values = [handle_single_value(v) for v in values]
-        values = handle_single_value(values)
+        else:
+            values = handle_single_value(values)
         if is_list_target and not isinstance(values, (tuple, list)):
             values = [values]
         elif not is_list_target and isinstance(values, (tuple, list)):
diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py
index c847aca2a0b98..26e3c721a0e29 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -1354,8 +1354,9 @@ def get_filters(cls, raw_filters, num_cols):  # noqa
                 continue
             cond = None
             is_numeric_col = col in num_cols
+            is_list_target = op in ('in', 'not in')
             eq = cls.filter_values_handler(
-                eq, is_list_target=op in ('in', 'not in'),
+                eq, is_list_target=is_list_target,
                 target_column_is_numeric=is_numeric_col)
             if op == '==':
                 cond = Dimension(col) == eq
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 9df593d2562e9..86528a3c06879 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -578,10 +578,11 @@ def get_sqla_query(  # sqla
             col = flt['col']
             op = flt['op']
             col_obj = cols.get(col)
+            is_list_target = op in ('in', 'not in')
             eq = self.filter_values_handler(
                 flt.get('val'),
                 target_column_is_numeric=col_obj.is_num,
-                is_list_target=op in ('in', 'not in'))
+                is_list_target=is_list_target)
             if col_obj:
                 if op in ('in', 'not in'):
                     cond = col_obj.sqla_col.in_(eq)
diff --git a/superset/viz.py b/superset/viz.py
index 5eef1940752e4..5e6042dc46730 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -1700,8 +1700,6 @@ def run_extra_queries(self):
         for flt in filters:
             qry['groupby'] = [flt]
             df = self.get_df_payload(query_obj=qry).get('df')
-            print(df)
-            print(df.dtypes)
             self.dataframes[flt] = df
 
     def filter_query_obj(self):
diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py
index b750d3d87367d..c367bd7ad7bd7 100644
--- a/tests/druid_func_tests.py
+++ b/tests/druid_func_tests.py
@@ -34,7 +34,7 @@ class DruidFuncTestCase(unittest.TestCase):
     def test_get_filters_ignores_invalid_filter_objects(self):
         filtr = {'col': 'col1', 'op': '=='}
         filters = [filtr]
-        self.assertEqual(None, DruidDatasource.get_filters(filters, []))
+        self.assertIsNone(DruidDatasource.get_filters(filters, []))
 
     def test_get_filters_constructs_filter_in(self):
         filtr = {'col': 'A', 'op': 'in', 'val': ['a', 'b', 'c']}
@@ -108,7 +108,7 @@ def test_get_filters_ignores_in_not_in_with_empty_value(self):
         filtr1 = {'col': 'A', 'op': 'in', 'val': []}
         filtr2 = {'col': 'A', 'op': 'not in', 'val': []}
         res = DruidDatasource.get_filters([filtr1, filtr2], [])
-        self.assertEqual(None, res)
+        self.assertIsNone(res)
 
     def test_get_filters_constructs_equals_for_in_not_in_single_value(self):
         filtr = {'col': 'A', 'op': 'in', 'val': ['a']}
@@ -122,12 +122,12 @@ def test_get_filters_handles_arrays_for_string_types(self):
 
         filtr = {'col': 'A', 'op': '==', 'val': []}
         res = DruidDatasource.get_filters([filtr], [])
-        self.assertEqual(None, res.filter['filter']['value'])
+        self.assertIsNone(res.filter['filter']['value'])
 
     def test_get_filters_handles_none_for_string_types(self):
         filtr = {'col': 'A', 'op': '==', 'val': None}
         res = DruidDatasource.get_filters([filtr], [])
-        self.assertEqual(None, res)
+        self.assertIsNone(res)
 
     def test_get_filters_extracts_values_in_quotes(self):
         filtr = {'col': 'A', 'op': 'in', 'val': ['  "a" ']}