Bug Fixes

- #40 - #41 - "Open Popup" button for ipython iframes - column width resizing on sorting - additional int/float descriptors (sum, median, mode, var, sem, skew, kurt) - wordcloud chart type
man-group · Dec 23, 2019 · aeaadac · aeaadac
1 parent faeee73
commit aeaadac
Show file tree

Hide file tree

Showing 19 changed files with 520 additions and 34 deletions.
diff --git a/dtale/views.py b/dtale/views.py
@@ -604,7 +604,7 @@ def dtypes(data_id):
         return jsonify(error=str(e), traceback=str(traceback.format_exc()))
 
 
-def load_describe(column_series):
+def load_describe(column_series, additional_aggs=None):
     """
     Helper function for grabbing the output from :meth:`pandas:pandas.Series.describe` in a JSON serializable format
 
@@ -613,6 +613,13 @@ def load_describe(column_series):
     :return: JSON serializable dictionary of the output from calling :meth:`pandas:pandas.Series.describe`
     """
     desc = column_series.describe().to_frame().T
+    if additional_aggs:
+        for agg in additional_aggs:
+            if agg == 'mode':
+                mode = column_series.mode().values
+                desc['mode'] = np.nan if len(mode) > 1 else mode[0]
+                continue
+            desc[agg] = getattr(column_series, agg)()
     desc_f_overrides = {
         'I': lambda f, i, c: f.add_int(i, c, as_string=True),
         'F': lambda f, i, c: f.add_float(i, c, precision=4, as_string=True),
@@ -645,14 +652,29 @@ def describe(data_id, column):
     """
     try:
         data = DATA[data_id]
-        desc = load_describe(data[column])
+        additional_aggs = None
+        dtype = next((dtype_info['dtype'] for dtype_info in DTYPES[data_id] if dtype_info['name'] == column), None)
+        if classify_type(dtype) in ['I', 'F']:
+            additional_aggs = ['sum', 'median', 'mode', 'var', 'sem', 'skew', 'kurt']
+        desc = load_describe(data[column], additional_aggs=additional_aggs)
         return_data = dict(describe=desc, success=True)
         uniq_vals = data[column].unique()
         if 'unique' not in return_data['describe']:
             return_data['describe']['unique'] = json_int(len(uniq_vals), as_string=True)
         if len(uniq_vals) <= 100:
             uniq_f = find_dtype_formatter(get_dtypes(data)[column])
-            return_data['uniques'] = [uniq_f(u, nan_display='N/A') for u in uniq_vals]
+            return_data['uniques'] = dict(
+                data=[uniq_f(u, nan_display='N/A') for u in uniq_vals],
+                top=False
+            )
+        else:  # get top 100 most common values
+            uniq_vals = data[column].value_counts().sort_values(ascending=False).head(100).index.values
+            uniq_f = find_dtype_formatter(get_dtypes(data)[column])
+            return_data['uniques'] = dict(
+                data=[uniq_f(u, nan_display='N/A') for u in uniq_vals],
+                top=True
+            )
+
         return jsonify(return_data)
     except BaseException as e:
         return jsonify(dict(error=str(e), traceback=str(traceback.format_exc())))

diff --git a/package.json b/package.json
@@ -142,6 +142,7 @@
     "react-redux": "7.1.3",
     "react-select": "3.0.8",
     "react-virtualized": "9.21.2",
+    "react-wordcloud": "1.1.1",
     "redux": "4.0.4",
     "redux-thunk": "2.3.0",
     "serialize-javascript": "2.1.1",

diff --git a/static/__tests__/iframe/DataViewer-within-iframe-test.jsx b/static/__tests__/iframe/DataViewer-within-iframe-test.jsx
@@ -7,7 +7,7 @@ import { DataViewerMenu } from "../../dtale/DataViewerMenu";
 import mockPopsicle from "../MockPopsicle";
 import * as t from "../jest-assertions";
 import reduxUtils from "../redux-test-utils";
-import { buildInnerHTML, withGlobalJquery } from "../test-utils";
+import { buildInnerHTML, clickMainMenuButton, withGlobalJquery } from "../test-utils";
 
 const originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetHeight");
 const originalOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetWidth");
@@ -29,7 +29,7 @@ describe("DataViewer within iframe tests", () => {
     delete window.open;
     delete window.top;
     delete window.self;
-    window.location = { reload: jest.fn() };
+    window.location = { reload: jest.fn(), pathname: "/dtale/iframe/1" };
     window.open = jest.fn();
     window.top = { location: { href: "http://test.com" } };
     window.self = { location: { href: "http://test/dtale/iframe" } };
@@ -86,10 +86,12 @@ describe("DataViewer within iframe tests", () => {
           .map(s => s.text()),
         _.concat(
           ["Describe", "Filter", "Correlations", "Charts", "Resize", "Heat Map", "Instances 1", "About"],
-          ["Refresh", "Shutdown"]
+          ["Refresh", "Open Popup", "Shutdown"]
         ),
         "Should render default iframe menu options"
       );
+      clickMainMenuButton(result, "Open Popup");
+      expect(window.open.mock.calls[window.open.mock.calls.length - 1][0]).toBe("/dtale/iframe/1");
       done();
     }, 600);
   });

diff --git a/static/__tests__/popups/ChartsBody-test.jsx b/static/__tests__/popups/ChartsBody-test.jsx
@@ -43,7 +43,7 @@ describe("ChartsBody tests", () => {
     result.update();
     setTimeout(() => {
       result.update();
-      t.ok(_.includes(result.html(), "No data found."), "shoudl render no data message");
+      t.ok(_.includes(result.html(), "No data found."), "should render no data message");
       done();
     }, 200);
   });

diff --git a/static/__tests__/popups/Correlations-test.jsx b/static/__tests__/popups/Correlations-test.jsx
@@ -54,6 +54,7 @@ describe("Correlations tests", () => {
       const chartCfg = { ctx, cfg, data: cfg.data, destroyed: false };
       chartCfg.destroy = () => (chartCfg.destroyed = true);
       chartCfg.getElementsAtXAxis = _evt => [{ _index: 0 }];
+      chartCfg.getElementAtEvent = _evt => [{ _datasetIndex: 0, _index: 0, _chart: { config: cfg, data: cfg.data } }];
       return chartCfg;
     });
 
@@ -169,7 +170,13 @@ describe("Correlations tests", () => {
   test("Correlations rendering data w/ no date columns", done => {
     const Correlations = require("../../popups/Correlations").ReactCorrelations;
     buildInnerHTML({ settings: "" });
-    const result = mount(<Correlations chartData={_.assign({}, chartData, { query: "no-date" })} dataId="1" />, {
+    const props = {
+      chartData: _.assign({}, chartData, { query: "no-date" }),
+      dataId: "1",
+      onClose: _.noop,
+      propagateState: _.noop,
+    };
+    const result = mount(<Correlations {...props} />, {
       attachTo: document.getElementById("content"),
     });
     result.update();
@@ -194,6 +201,12 @@ describe("Correlations tests", () => {
           scatterChart.data
         );
         t.deepEqual(label, ["col1: NaN", "col2: 1.5"], "should render label");
+        scatterChart.cfg.options.onClick({});
+        const corr = result.instance();
+
+        t.ok(corr.shouldComponentUpdate(_.assignIn({ foo: 1 }, corr.props)), "should update");
+        t.ok(!corr.shouldComponentUpdate(corr.props, _.assignIn({}, corr.state, { chart: null })), "shouldn't update");
+        t.ok(!corr.shouldComponentUpdate(corr.props, corr.state), "shouldn't update");
         done();
       }, 200);
     }, 200);

diff --git a/static/__tests__/popups/WordcloudBody-test.jsx b/static/__tests__/popups/WordcloudBody-test.jsx
@@ -0,0 +1,81 @@
+import { mount } from "enzyme";
+import _ from "lodash";
+import React from "react";
+
+import mockPopsicle from "../MockPopsicle";
+import * as t from "../jest-assertions";
+import { withGlobalJquery } from "../test-utils";
+
+describe("WordcloudBody tests", () => {
+  beforeAll(() => {
+    const mockBuildLibs = withGlobalJquery(() =>
+      mockPopsicle.mock(url => {
+        if (url.startsWith("chart-data-error-test1")) {
+          return { data: {} };
+        }
+        if (url.startsWith("chart-data-error-test2")) {
+          return { error: "Error test." };
+        }
+        const { urlFetcher } = require("../redux-test-utils").default;
+        return urlFetcher(url);
+      })
+    );
+
+    const mockChartUtils = withGlobalJquery(() => (ctx, cfg) => {
+      const chartCfg = { ctx, cfg, data: cfg.data, destroyed: false };
+      chartCfg.destroy = () => (chartCfg.destroyed = true);
+      chartCfg.getElementsAtXAxis = _evt => [{ _index: 0 }];
+      return chartCfg;
+    });
+
+    const mockD3Cloud = withGlobalJquery(() => () => {
+      const cloudCfg = {};
+      const propUpdate = prop => val => {
+        cloudCfg[prop] = val;
+        return cloudCfg;
+      };
+      cloudCfg.size = propUpdate("size");
+      cloudCfg.padding = propUpdate("padding");
+      cloudCfg.words = propUpdate("words");
+      cloudCfg.rotate = propUpdate("rotate");
+      cloudCfg.spiral = propUpdate("spiral");
+      cloudCfg.random = propUpdate("random");
+      cloudCfg.text = propUpdate("text");
+      cloudCfg.font = propUpdate("font");
+      cloudCfg.fontStyle = propUpdate("fontStyle");
+      cloudCfg.fontWeight = propUpdate("fontWeight");
+      cloudCfg.fontSize = () => ({
+        on: () => ({ start: _.noop }),
+      });
+      return cloudCfg;
+    });
+
+    jest.mock("popsicle", () => mockBuildLibs);
+    jest.mock("d3-cloud", () => mockD3Cloud);
+    jest.mock("chart.js", () => mockChartUtils);
+    jest.mock("chartjs-plugin-zoom", () => ({}));
+    jest.mock("chartjs-chart-box-and-violin-plot/build/Chart.BoxPlot.js", () => ({}));
+  });
+
+  test("WordcloudBody missing data", done => {
+    const WordcloudBody = require("../../popups/charts/WordcloudBody").default;
+
+    const result = mount(<WordcloudBody chartType="wordcloud" data={{}} />, {
+      attachTo: document.getElementById("content"),
+    });
+    result.update();
+    t.notOk(result.html(), "shouldn't render anything");
+    done();
+  });
+
+  test("WordcloudBody invalid chartType type", done => {
+    const WordcloudBody = require("../../popups/charts/WordcloudBody").default;
+
+    const result = mount(<WordcloudBody chartType="bar" data={{}} />, {
+      attachTo: document.getElementById("content"),
+    });
+    result.update();
+    t.notOk(result.html(), "shouldn't render anything");
+    done();
+  });
+});
diff --git a/static/__tests__/popups/window/Charts-test.jsx b/static/__tests__/popups/window/Charts-test.jsx
@@ -1,3 +1,5 @@
+import qs from "querystring";
+
 import { mount } from "enzyme";
 import _ from "lodash";
 import React from "react";
@@ -23,6 +25,10 @@ describe("Charts tests", () => {
 
     const mockBuildLibs = withGlobalJquery(() =>
       mockPopsicle.mock(url => {
+        const urlParams = qs.parse(url.split("?")[1]);
+        if (urlParams.x === "error" && urlParams.y === "error2") {
+          return { data: {} };
+        }
         const { urlFetcher } = require("../../redux-test-utils").default;
         return urlFetcher(url);
       })
@@ -37,7 +43,30 @@ describe("Charts tests", () => {
       return chartCfg;
     });
 
+    const mockD3Cloud = withGlobalJquery(() => () => {
+      const cloudCfg = {};
+      const propUpdate = prop => val => {
+        cloudCfg[prop] = val;
+        return cloudCfg;
+      };
+      cloudCfg.size = propUpdate("size");
+      cloudCfg.padding = propUpdate("padding");
+      cloudCfg.words = propUpdate("words");
+      cloudCfg.rotate = propUpdate("rotate");
+      cloudCfg.spiral = propUpdate("spiral");
+      cloudCfg.random = propUpdate("random");
+      cloudCfg.text = propUpdate("text");
+      cloudCfg.font = propUpdate("font");
+      cloudCfg.fontStyle = propUpdate("fontStyle");
+      cloudCfg.fontWeight = propUpdate("fontWeight");
+      cloudCfg.fontSize = () => ({
+        on: () => ({ start: _.noop }),
+      });
+      return cloudCfg;
+    });
+
     jest.mock("popsicle", () => mockBuildLibs);
+    jest.mock("d3-cloud", () => mockD3Cloud);
     jest.mock("chart.js", () => mockChartUtils);
     jest.mock("chartjs-plugin-zoom", () => ({}));
     jest.mock("chartjs-chart-box-and-violin-plot/build/Chart.BoxPlot.js", () => ({}));
@@ -123,6 +152,11 @@ describe("Charts tests", () => {
           .instance()
           .onChange({ value: "bar" });
         result.update();
+        filters
+          .last()
+          .instance()
+          .onChange({ value: "wordcloud" });
+        result.update();
         filters
           .last()
           .instance()
@@ -163,6 +197,16 @@ describe("Charts tests", () => {
             "val1: 1.1235",
             "should render tooltip label"
           );
+          filters
+            .last()
+            .instance()
+            .onChange({ value: "wordcloud" });
+          result.update();
+          filters
+            .last()
+            .instance()
+            .onChange({ value: "line" });
+          result.update();
           result
             .find(Charts)
             .find("input")
@@ -180,9 +224,63 @@ describe("Charts tests", () => {
             1.1235,
             "should render tooltip label"
           );
+          filters
+            .last()
+            .instance()
+            .onChange({ value: "wordcloud" });
+          result.update();
+          const cb = result.find(ChartsBody).instance();
+          t.notOk(cb.shouldComponentUpdate(cb.props, cb.state), "shouldn't update chart body");
+          t.ok(
+            cb.shouldComponentUpdate(cb.props, _.assignIn({}, cb.state, { error: "test" })),
+            "should update chart body"
+          );
+          t.ok(cb.shouldComponentUpdate(cb.props, _.assignIn({}, cb.state, { data: {} })), "should update chart body");
+          t.notOk(
+            cb.shouldComponentUpdate(cb.props, _.assignIn({}, cb.state, { chart: true })),
+            "shouldn't update chart body"
+          );
           done();
         }, 400);
       }, 400);
     }, 600);
   });
+
+  test("Charts: rendering empty data", done => {
+    const Charts = require("../../../popups/charts/Charts").ReactCharts;
+    const ChartsBody = require("../../../popups/charts/ChartsBody").default;
+    buildInnerHTML({ settings: "" });
+    const result = mount(<Charts chartData={{ visible: true }} dataId="1" />, {
+      attachTo: document.getElementById("content"),
+    });
+
+    setTimeout(() => {
+      result.update();
+      let filters = result.find(Charts).find(Select);
+      filters
+        .first()
+        .instance()
+        .onChange({ value: "error" });
+      filters
+        .at(1)
+        .instance()
+        .onChange({ value: "error2" });
+      result
+        .find(Charts)
+        .find("button")
+        .first()
+        .simulate("click");
+      setTimeout(() => {
+        result.update();
+        filters = result.find(Charts).find(Select);
+        filters
+          .last()
+          .instance()
+          .onChange({ value: "bar" });
+        result.update();
+        t.ok(result.find(ChartsBody).instance().state.charts === null, "should not render chart");
+        done();
+      }, 400);
+    }, 400);
+  });
 });
diff --git a/static/__tests__/redux-test-utils.jsx b/static/__tests__/redux-test-utils.jsx
@@ -54,6 +54,7 @@ const DESCRIBE = {
       "50%": 2.5,
       "75%": 4,
     },
+    uniques: { data: [1, 2, 3, 4], top: true },
   },
   col2: {
     describe: {
@@ -70,7 +71,7 @@ const DESCRIBE = {
   },
   col3: {
     describe: { count: 4, freq: 4, top: "foo", unique: 1 },
-    uniques: ["foo"],
+    uniques: { data: ["foo"], top: false },
   },
   col4: {
     describe: {

diff --git a/static/dtale/DataViewer.jsx b/static/dtale/DataViewer.jsx
@@ -149,7 +149,15 @@ class ReactDataViewer extends React.Component {
           );
           newState.columns = _.concat(columns, newCols);
         }
-        this.setState(newState);
+        let callback = _.noop;
+        if (refresh) {
+          callback = () =>
+            this.setState({
+              columns: _.map(this.state.columns, c => _.assignIn(c, { width: gu.calcColWidth(c, this.state) })),
+              triggerResize: true,
+            });
+        }
+        this.setState(newState, callback);
       })
       .catch((e, callstack) => {
         logException(e, callstack);