Specify the metric to order by for Series Limit (#1351)

caravel/forms.py

@@ -544,6 +544,11 @@ def __init__(self, viz):
                 "description": _(
                     "Limits the number of time series that get displayed")
             }),
+            'timeseries_limit_metric': (SelectField, {
+                "label": _("Sort By"),
+                "choices": datasource.metrics_combo,
+                "description": _("Metric used to define the top series")
+            }),
             'rolling_type': (SelectField, {
                 "label": _("Rolling"),
                 "default": 'None',

caravel/models.py

@@ -867,8 +867,11 @@ def query(  # sqla
             from_dttm, to_dttm,
             filter=None,  # noqa
             is_timeseries=True,
-            timeseries_limit=15, row_limit=None,
-            inner_from_dttm=None, inner_to_dttm=None,
+            timeseries_limit=15,
+            timeseries_limit_metric=None,
+            row_limit=None,
+            inner_from_dttm=None,
+            inner_to_dttm=None,
             orderby=None,
             extras=None,
             columns=None):
@@ -887,7 +890,11 @@ def query(  # sqla
                 "and is required by this type of chart"))
 
         metrics_exprs = [metrics_dict.get(m).sqla_col for m in metrics]
-
+        timeseries_limit_metric = metrics_dict.get(timeseries_limit_metric)
+        timeseries_limit_metric_expr = None
+        if timeseries_limit_metric:
+            timeseries_limit_metric_expr = \
+                timeseries_limit_metric.sqla_col
         if metrics:
             main_metric_expr = metrics_exprs[0]
         else:
@@ -1023,7 +1030,10 @@ def visit_column(element, compiler, **kw):
             subq = subq.select_from(tbl)
             subq = subq.where(and_(*(where_clause_and + inner_time_filter)))
             subq = subq.group_by(*inner_groupby_exprs)
-            subq = subq.order_by(desc(main_metric_expr))
+            ob = main_metric_expr
+            if timeseries_limit_metric_expr is not None:
+                ob = timeseries_limit_metric_expr
+            subq = subq.order_by(desc(ob))
             subq = subq.limit(timeseries_limit)
             on_clause = []
             for i, gb in enumerate(groupby):
@@ -1689,6 +1699,7 @@ def query(  # druid
             filter=None,  # noqa
             is_timeseries=True,
             timeseries_limit=None,
+            timeseries_limit_metric=None,
             row_limit=None,
             inner_from_dttm=None, inner_to_dttm=None,
             orderby=None,
@@ -1794,6 +1805,9 @@ def recursive_get_fields(_conf):
         client = self.cluster.get_pydruid_client()
         orig_filters = filters
         if timeseries_limit and is_timeseries:
+            order_by = metrics[0] if metrics else self.metrics[0]
+            if timeseries_limit_metric:
+                order_by = timeseries_limit_metric
             # Limit on the number of timeseries, doing a two-phases query
             pre_qry = deepcopy(qry)
             pre_qry['granularity'] = "all"
@@ -1804,7 +1818,7 @@ def recursive_get_fields(_conf):
                     inner_from_dttm.isoformat() + '/' +
                     inner_to_dttm.isoformat()),
                 "columns": [{
-                    "dimension": metrics[0] if metrics else self.metrics[0],
+                    "dimension": order_by,
                     "direction": "descending",
                 }],
             }

caravel/viz.py

@@ -240,6 +240,7 @@ def query_obj(self):
             form_data.get("granularity") or form_data.get("granularity_sqla")
         )
         limit = int(form_data.get("limit", 0))
+        timeseries_limit_metric = form_data.get("timeseries_limit_metric")
         row_limit = int(
             form_data.get("row_limit", config.get("ROW_LIMIT")))
         since = (
@@ -275,6 +276,7 @@ def query_obj(self):
             'filter': self.query_filters(),
             'timeseries_limit': limit,
             'extras': extras,
+            'timeseries_limit_metric': timeseries_limit_metric,
         }
         return d
 
@@ -999,7 +1001,8 @@ class NVD3TimeSeriesViz(NVD3Viz):
         'label': None,
         'fields': (
             'metrics',
-            'groupby', 'limit',
+            'groupby',
+            ('limit', 'timeseries_limit_metric'),
         ),
     }, {
         'label': _('Chart Options'),

run_specific_test.sh

@@ -5,4 +5,4 @@ export CARAVEL_CONFIG=tests.caravel_test_config
 set -e
 caravel/bin/caravel version -v
 export SOLO_TEST=1
-nosetests tests.core_tests:CoreTests.test_slices
+nosetests tests.core_tests:CoreTests