fixes for [#50](#50)

dtale/utils.py

@@ -111,6 +111,26 @@ def get_str_arg(r, name, default=None):
             return default
 
 
+def get_json_arg(r, name, default=None):
+    """
+    Retrieve argument from :attr:`flask:flask.request` and parse JSON to python data structure
+
+    :param r: :attr:`flask:flask.request`
+    :param name: argument name
+    :type: str
+    :param default: default value if parameter is non-existent, defaults to None
+    :return: parsed JSON
+    """
+    val = r.args.get(name)
+    if val is None or val == '':
+        return default
+    else:
+        try:
+            return json.loads(val)
+        except BaseException:
+            return default
+
+
 def get_int_arg(r, name, default=None):
     """
     Retrieve argument from :attr:`flask:flask.request` and convert to integer

dtale/views.py

@@ -17,7 +17,7 @@
 from dtale.utils import (build_shutdown_url, classify_type, dict_merge,
                          filter_df_for_grid, find_dtype_formatter,
                          find_selected_column, get_bool_arg, get_dtypes,
-                         get_int_arg, get_str_arg, grid_columns,
+                         get_int_arg, get_json_arg, get_str_arg, grid_columns,
                          grid_formatter, json_date, json_float, json_int,
                          json_timestamp, jsonify, make_list,
                          retrieve_grid_params, running_with_flask_debug,
@@ -539,7 +539,7 @@ def update_settings(data_id):
         global SETTINGS
 
         curr_settings = SETTINGS.get(data_id, {})
-        updated_settings = dict_merge(curr_settings, json.loads(get_str_arg(request, 'settings', '{}')))
+        updated_settings = dict_merge(curr_settings, get_json_arg(request, 'settings', {}))
         SETTINGS[data_id] = updated_settings
         return jsonify(dict(success=True))
     except BaseException as e:
@@ -718,10 +718,8 @@ def get_data(data_id):
             DTYPES[data_id] = build_dtypes_state(data)
 
         params = retrieve_grid_params(request)
-        ids = get_str_arg(request, 'ids')
-        if ids:
-            ids = json.loads(ids)
-        else:
+        ids = get_json_arg(request, 'ids')
+        if ids is None:
             return jsonify({})
 
         col_types = DTYPES[data_id]
@@ -861,7 +859,7 @@ def build_chart(data, x, y, group_col=None, agg=None, allow_duplicates=False, **
     :param x: column to be used as x-axis of chart
     :type x: str
     :param y: column to be used as y-axis of chart
-    :type y: str
+    :type y: list of strings
     :param group: comma-separated string of columns to group chart data by
     :type group: str, optional
     :param aggregation: points to a specific function that can be applied to
@@ -880,7 +878,7 @@ def build_formatters(df):
         return data_f, range_f
 
     x_col = str('x')
-    y_cols = y.split(',')
+    y_cols = make_list(y)
     if group_col is not None:
         data = data[group_col + [x] + y_cols].sort_values(group_col + [x])
         y_cols = [str(y_col) for y_col in y_cols]
@@ -973,10 +971,8 @@ def get_chart_data(data_id):
             if not len(data):
                 return jsonify(dict(error='query "{}" found no data, please alter'.format(query)))
         x = get_str_arg(request, 'x')
-        y = get_str_arg(request, 'y')
-        group_col = get_str_arg(request, 'group')
-        if group_col is not None:
-            group_col = group_col.split(',')
+        y = get_json_arg(request, 'y')
+        group_col = get_json_arg(request, 'group')
         agg = get_str_arg(request, 'agg')
         allow_duplicates = get_bool_arg(request, 'allowDupes')
         window = get_int_arg(request, 'rollingWin')
@@ -1009,12 +1005,12 @@ def get_correlations_ts(data_id):
         data = DATA[data_id]
         data = data.query(query) if query is not None else data
         cols = get_str_arg(request, 'cols')
-        cols = cols.split(',')
+        cols = json.loads(cols)
         date_col = get_str_arg(request, 'dateCol')
         rolling_window = get_int_arg(request, 'rollingWindow')
         if rolling_window:
             [col1, col2] = list(set(cols))
-            data = data[['date', col1, col2]].set_index('date')
+            data = data[[date_col, col1, col2]].set_index(date_col)
             data = data[col1].rolling(rolling_window).corr(data[col2]).reset_index()
         else:
             data = data.groupby(date_col)[list(set(cols))].corr(method='pearson')
@@ -1054,19 +1050,29 @@ def get_scatter(data_id):
         y: col2
     } or {error: 'Exception message', traceback: 'Exception stacktrace'}
     """
-    cols = get_str_arg(request, 'cols')
-    cols = cols.split(',')
+    cols = get_json_arg(request, 'cols')
     query = get_str_arg(request, 'query')
     date = get_str_arg(request, 'date')
     date_col = get_str_arg(request, 'dateCol')
+    rolling = get_bool_arg(request, 'rolling')
     try:
         data = DATA[data_id]
-        data = data[data[date_col] == date] if date else data
         if query:
             data = data.query(query)
 
-        data = data[list(set(cols))].dropna(how='any')
-        data[str('index')] = data.index
+        idx_col = str('index')
+        y_cols = [cols[1], idx_col]
+        if rolling:
+            window = get_int_arg(request, 'window')
+            idx = min(data[data[date_col] == date].index) + 1
+            data = data.iloc[(idx - window):idx]
+            data = data[list(set(cols)) + [date_col]].dropna(how='any')
+            y_cols.append(date_col)
+        else:
+            data = data[data[date_col] == date] if date else data
+            data = data[list(set(cols))].dropna(how='any')
+
+        data[idx_col] = data.index
         s0 = data[cols[0]]
         s1 = data[cols[1]]
         pearson = s0.corr(s1, method='pearson')
@@ -1084,7 +1090,7 @@ def get_scatter(data_id):
                 stats=stats,
                 error='Dataset exceeds 15,000 records, cannot render scatter. Please apply filter...'
             )
-        data = build_chart(data, cols[0], str('{},index'.format(cols[1])), allow_duplicates=True)
+        data = build_chart(data, cols[0], y_cols, allow_duplicates=True)
         data['x'] = cols[0]
         data['y'] = cols[1]
         data['stats'] = stats

static/__tests__/actions/url-utils-test.jsx

@@ -14,7 +14,7 @@ describe("url-utils tests", () => {
     t.deepEqual(
       urlParams,
       {
-        cols: "col1,col2",
+        cols: "[\"col1\",\"col2\"]",
         query: "col == 3",
         sort: '[["col1","ASC"]]',
         ids: "[0,5]",

static/__tests__/popups/Correlations-test.jsx

@@ -199,7 +199,7 @@ describe("Correlations tests", () => {
           [{ datasetIndex: 0, index: 0 }],
           scatterChart.data
         );
-        t.deepEqual(title, [["Index: 0"]], "should render title");
+        t.deepEqual(title, ["index: 0"], "should render title");
         const label = scatterChart.cfg.options.tooltips.callbacks.label(
           { datasetIndex: 0, index: 0 },
           scatterChart.data

static/__tests__/popups/window/Charts-bar-test.jsx

@@ -37,7 +37,7 @@ describe("Charts bar tests", () => {
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
         const urlParams = qs.parse(url.split("?")[1]);
-        if (urlParams.x === "error" && urlParams.y === "error2") {
+        if (urlParams.x === "error" && _.includes(JSON.parse(urlParams.y), "error2")) {
           return { data: {} };
         }
         const { urlFetcher } = require("../../redux-test-utils").default;

static/__tests__/popups/window/Charts-multi-column-test.jsx

@@ -38,7 +38,7 @@ describe("Charts tests", () => {
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
         const urlParams = qs.parse(url.split("?")[1]);
-        if (urlParams.x === "error" && urlParams.y === "error2") {
+        if (urlParams.x === "error" && _.includes(JSON.parse(urlParams.y), "error2")) {
           return { data: {} };
         }
         const { urlFetcher } = require("../../redux-test-utils").default;
@@ -139,11 +139,12 @@ describe("Charts tests", () => {
       setTimeout(() => {
         result.update();
         t.ok(result.find(ChartsBody).instance().state.charts.length == 1, "should render charts");
-        t.ok(
-          _.endsWith(
-            result.find(Charts).instance().state.url,
-            "x=col4&y=col1%2Ccol2&query=col4%20%3D%3D%20'20181201'&agg=rolling&rollingWin=10&rollingComp=corr"
-          ),
+        t.equal(
+          _.last(_.split(result.find(Charts).instance().state.url, "?")),
+          _.join([
+            "x=col4&y=%5B%22col1%22%2C%22col2%22%5D&query=col4%20%3D%3D%20'20181201'",
+            "agg=rolling&rollingWin=10&rollingComp=corr",
+          ], "&"),
           "should update chart URL"
         );
         result

static/__tests__/popups/window/Charts-rolling-test.jsx

@@ -27,7 +27,7 @@ describe("Charts rolling tests", () => {
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
         const urlParams = qs.parse(url.split("?")[1]);
-        if (urlParams.x === "error" && urlParams.y === "error2") {
+        if (urlParams.x === "error" && _.includes(JSON.parse(urlParams.y), "error2")) {
           return { data: {} };
         }
         const { urlFetcher } = require("../../redux-test-utils").default;

static/__tests__/popups/window/Charts-scatter-test.jsx

@@ -35,7 +35,7 @@ describe("Charts scatter tests", () => {
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
         const urlParams = qs.parse(url.split("?")[1]);
-        if (urlParams.x === "error" && urlParams.y === "error2") {
+        if (urlParams.x === "error" && _.includes(JSON.parse(urlParams.y), "error2")) {
           return { data: {} };
         }
         const { urlFetcher } = require("../../redux-test-utils").default;

static/__tests__/popups/window/Charts-test.jsx

@@ -39,7 +39,7 @@ describe("Charts tests", () => {
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
         const urlParams = qs.parse(url.split("?")[1]);
-        if (urlParams.x === "error" && urlParams.y === "error2") {
+        if (urlParams.x === "error" && _.includes(JSON.parse(urlParams.y), "error2")) {
           return { data: {} };
         }
         if (_.startsWith(url, "/dtale/dtypes/9")) {
@@ -143,11 +143,9 @@ describe("Charts tests", () => {
       setTimeout(() => {
         result.update();
         t.ok(result.find(ChartsBody).instance().state.charts.length == 1, "should render charts");
-        t.ok(
-          _.endsWith(
-            result.find(Charts).instance().state.url,
-            "x=col4&y=col1&query=col4%20%3D%3D%20'20181201'&agg=rolling&rollingWin=10&rollingComp=corr"
-          ),
+        t.equal(
+          _.last(_.split(result.find(Charts).instance().state.url, "?")),
+          "x=col4&y=%5B%22col1%22%5D&query=col4%20%3D%3D%20'20181201'&agg=rolling&rollingWin=10&rollingComp=corr",
           "should update chart URL"
         );
         result

static/__tests__/redux-test-utils.jsx

@@ -140,14 +140,14 @@ function urlFetcher(url) {
     return scatterData;
   } else if (url.startsWith("/dtale/chart-data")) {
     if (urlParams.group) {
-      if (_.includes(urlParams.y, ",")) {
+      if (_.size(JSON.parse(urlParams.y)) > 1) {
         return _.assignIn({}, groupedChartsData, {
           data: _.mapValues(groupedChartsData.data, d => _.assignIn(d, { col2: d.col1 })),
         });
       }
       return groupedChartsData;
     }
-    if (_.includes(urlParams.y, ",")) {
+    if (_.size(JSON.parse(urlParams.y)) > 1) {
       return _.assignIn({}, chartsData, {
         data: _.mapValues(chartsData.data, d => _.assignIn(d, { col2: d.col1 })),
       });

static/actions/url-utils.js

@@ -13,7 +13,7 @@ const URL_KEYS = {
   ids: v => ({ ids: _.isEmpty(v) ? null : JSON.stringify(v) }),
   sortInfo: v => ({ sort: _.isEmpty(v) ? null : JSON.stringify(v) }),
   query: v => ({ query: v }),
-  selectedCols: v => ({ cols: _.isEmpty(v) ? null : _.join(v, ",") }),
+  selectedCols: v => ({ cols: _.isEmpty(v) ? null : JSON.stringify(v) }),
   tsColumns: v => ({ ts_columns: _.isEmpty(v) ? null : JSON.stringify(v) }),
 };
 

static/popups/Correlations.jsx

@@ -80,7 +80,6 @@ class ReactCorrelations extends React.Component {
           if (state.hasDate) {
             if (rolling) {
               this.buildTs([col1, col2], state.selectedDate, 4);
-              this.buildScatter([col1, col2]);
             } else {
               this.buildTs([col1, col2], state.selectedDate);
             }
@@ -130,8 +129,12 @@ class ReactCorrelations extends React.Component {
       params.dateCol = this.state.selectedDate;
       params.date = date;
     }
+    if (this.state.rolling) {
+      params.rolling = this.state.rolling;
+      params.window = this.state.window;
+    }
     const path = `${BASE_SCATTER_URL}/${this.props.dataId}`;
-    const scatterUrl = buildURL(path, params, ["selectedCols", "query", "date", "dateCol"]);
+    const scatterUrl = buildURL(path, params, ["selectedCols", "query", "date", "dateCol", "rolling", "window"]);
     if (this.state.scatterUrl === scatterUrl) {
       return;
     }
@@ -213,9 +216,7 @@ class ReactCorrelations extends React.Component {
                   id: "y-corr",
                 },
               ];
-              if (!this.state.rolling) {
-                config.options.onClick = this.viewScatter;
-              }
+              config.options.onClick = this.viewScatter;
               config.options.legend = { display: false };
               config.plugins = [chartUtils.gradientLinePlugin(corrUtils.colorScale, "y-corr", -1, 1)];
               return config;

static/popups/charts/Charts.jsx

@@ -15,9 +15,9 @@ function generateChartState(state, { dataId }) {
   if (_.isNull(x) || _.isNull(y)) {
     return { url: null };
   }
-  const params = { x: x.value, y: _.join(_.map(y, "value"), ","), query };
+  const params = { x: x.value, y: JSON.stringify(_.map(y, "value")), query };
   if (!_.isNull(group)) {
-    params.group = _.join(_.map(group, "value"), ",");
+    params.group = JSON.stringify(_.map(group, "value"));
   }
   if (!_.isNull(aggregation)) {
     params.agg = aggregation;

static/popups/correlations/CorrelationScatterStats.jsx

@@ -1,10 +1,28 @@
 import _ from "lodash";
+import moment from "moment";
 import PropTypes from "prop-types";
 import React from "react";
 
 import corrUtils from "./correlationsUtils";
 
 class CorrelationScatterStats extends React.Component {
+  constructor(props) {
+    super(props);
+    this.renderDescription = this.renderDescription.bind(this);
+  }
+
+  renderDescription() {
+    const [col0, col1] = this.props.selectedCols;
+    let dateStr = this.props.date ? ` for ${this.props.date}` : "";
+    if (this.props.rolling) {
+      const startDate = moment(this.props.date)
+        .subtract(this.props.window, "days")
+        .format("YYYYMMDD");
+      dateStr = ` for ${startDate}-${this.props.date}`;
+    }
+    return <b style={{ color: "black" }}>{`${col0} vs. ${col1}${dateStr}`}</b>;
+  }
+
   render() {
     const [col0, col1] = this.props.selectedCols;
     const stats = this.props.stats || {};
@@ -18,9 +36,7 @@ class CorrelationScatterStats extends React.Component {
     return [
       <div key={0} className="pt-5">
         <dl className="property-pair inline">
-          <dt>
-            <b style={{ color: "black" }}>{`${col0} vs. ${col1}${this.props.date ? ` for ${this.props.date}` : ""}`}</b>
-          </dt>
+          <dt>{this.renderDescription()}</dt>
         </dl>
         <dl className="property-pair inline">{pearsonInfo}</dl>
         <dl className="property-pair inline">{spearmanInfo}</dl>
@@ -52,6 +68,8 @@ CorrelationScatterStats.propTypes = {
     only_in_s0: PropTypes.number,
     only_in_s1: PropTypes.number,
   }),
+  rolling: PropTypes.bool,
+  window: PropTypes.number,
 };
 
 export default CorrelationScatterStats;

static/popups/correlations/CorrelationsCell.jsx

@@ -41,10 +41,7 @@ class CorrelationsCell extends React.Component {
     if (!corrOnItself) {
       if (hasDate) {
         if (this.props.rolling) {
-          props.onClick = () => {
-            this.props.buildTs([row.column, prop], selectedDate, this.props.window);
-            this.props.buildScatter([row.column, prop]);
-          };
+          props.onClick = () => this.props.buildTs([row.column, prop], selectedDate, this.props.window);
         } else {
           props.onClick = () => this.props.buildTs([row.column, prop], selectedDate);
         }