[GitHub] mistercrunch closed pull request #5026: [bugfix] Fix ZeroDivisionError and get metrics label with percent metrics

[GitBox <gi...@apache.org>]

mistercrunch closed pull request #5026: [bugfix] Fix ZeroDivisionError and get metrics label with percent metrics
URL: https://github.com/apache/incubator-superset/pull/5026
 
 
   

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/viz.py b/superset/viz.py
index 38e56807c3..39d3411456 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -573,6 +573,7 @@ def get_data(self, df):
 
         # Sum up and compute percentages for all percent metrics
         percent_metrics = fd.get('percent_metrics') or []
+        percent_metrics = [self.get_metric_label(m) for m in percent_metrics]
 
         if len(percent_metrics):
             percent_metrics = list(filter(lambda m: m in df, percent_metrics))
@@ -581,15 +582,18 @@ def get_data(self, df):
                 for m in percent_metrics
             }
             metric_percents = {
-                m: list(map(lambda a: a / metric_sums[m], df[m]))
+                m: list(map(
+                    lambda a: None if metric_sums[m] == 0 else a / metric_sums[m], df[m]))
                 for m in percent_metrics
             }
             for m in percent_metrics:
                 m_name = '%' + m
                 df[m_name] = pd.Series(metric_percents[m], name=m_name)
             # Remove metrics that are not in the main metrics list
+            metrics = fd.get('metrics', [])
+            metrics = [self.get_metric_label(m) for m in metrics]
             for m in filter(
-                lambda m: m not in fd['metrics'] and m in df.columns,
+                lambda m: m not in metrics and m in df.columns,
                 percent_metrics,
             ):
                 del df[m]
diff --git a/tests/viz_tests.py b/tests/viz_tests.py
index fb56581434..30c37c84f1 100644
--- a/tests/viz_tests.py
+++ b/tests/viz_tests.py
@@ -120,12 +120,22 @@ def test_cache_timeout(self):
 class TableVizTestCase(unittest.TestCase):
     def test_get_data_applies_percentage(self):
         form_data = {
-            'percent_metrics': ['sum__A', 'avg__B'],
-            'metrics': ['sum__A', 'count', 'avg__C'],
+            'percent_metrics': [{
+                'expressionType': 'SIMPLE',
+                'aggregate': 'SUM',
+                'label': 'SUM(value1)',
+                'column': {'column_name': 'value1', 'type': 'DOUBLE'},
+            }, 'avg__B'],
+            'metrics': [{
+                'expressionType': 'SIMPLE',
+                'aggregate': 'SUM',
+                'label': 'SUM(value1)',
+                'column': {'column_name': 'value1', 'type': 'DOUBLE'},
+            }, 'count', 'avg__C'],
         }
         datasource = Mock()
         raw = {}
-        raw['sum__A'] = [15, 20, 25, 40]
+        raw['SUM(value1)'] = [15, 20, 25, 40]
         raw['avg__B'] = [10, 20, 5, 15]
         raw['avg__C'] = [11, 22, 33, 44]
         raw['count'] = [6, 7, 8, 9]
@@ -137,29 +147,29 @@ def test_get_data_applies_percentage(self):
         # Check method correctly transforms data and computes percents
         self.assertEqual(set([
             'groupA', 'groupB', 'count',
-            'sum__A', 'avg__C',
-            '%sum__A', '%avg__B',
+            'SUM(value1)', 'avg__C',
+            '%SUM(value1)', '%avg__B',
         ]), set(data['columns']))
         expected = [
             {
                 'groupA': 'A', 'groupB': 'x',
-                'count': 6, 'sum__A': 15, 'avg__C': 11,
-                '%sum__A': 0.15, '%avg__B': 0.2,
+                'count': 6, 'SUM(value1)': 15, 'avg__C': 11,
+                '%SUM(value1)': 0.15, '%avg__B': 0.2,
             },
             {
                 'groupA': 'B', 'groupB': 'x',
-                'count': 7, 'sum__A': 20, 'avg__C': 22,
-                '%sum__A': 0.2, '%avg__B': 0.4,
+                'count': 7, 'SUM(value1)': 20, 'avg__C': 22,
+                '%SUM(value1)': 0.2, '%avg__B': 0.4,
             },
             {
                 'groupA': 'C', 'groupB': 'y',
-                'count': 8, 'sum__A': 25, 'avg__C': 33,
-                '%sum__A': 0.25, '%avg__B': 0.1,
+                'count': 8, 'SUM(value1)': 25, 'avg__C': 33,
+                '%SUM(value1)': 0.25, '%avg__B': 0.1,
             },
             {
                 'groupA': 'C', 'groupB': 'z',
-                'count': 9, 'sum__A': 40, 'avg__C': 44,
-                '%sum__A': 0.40, '%avg__B': 0.3,
+                'count': 9, 'SUM(value1)': 40, 'avg__C': 44,
+                '%SUM(value1)': 0.40, '%avg__B': 0.3,
             },
         ]
         self.assertEqual(expected, data['records'])


 

----------------------------------------------------------------
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