From aeaadacdc6fc8ed40af9093ee5d42175de1aa4e9 Mon Sep 17 00:00:00 2001 From: Andrew Schonfeld Date: Sat, 21 Dec 2019 11:37:51 -0500 Subject: [PATCH] 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 --- dtale/views.py | 28 ++- package.json | 1 + .../iframe/DataViewer-within-iframe-test.jsx | 8 +- static/__tests__/popups/ChartsBody-test.jsx | 2 +- static/__tests__/popups/Correlations-test.jsx | 15 +- .../__tests__/popups/WordcloudBody-test.jsx | 81 +++++++++ .../__tests__/popups/window/Charts-test.jsx | 98 +++++++++++ static/__tests__/redux-test-utils.jsx | 3 +- static/dtale/DataViewer.jsx | 10 +- static/dtale/DataViewerMenu.jsx | 18 +- static/dtale/gridUtils.jsx | 5 +- static/popups/Correlations.jsx | 10 +- static/popups/Describe.jsx | 16 +- static/popups/charts/Charts.jsx | 5 +- static/popups/charts/ChartsBody.jsx | 27 ++- static/popups/charts/WordcloudBody.jsx | 51 ++++++ static/popups/describe/DtypesGrid.jsx | 3 +- tests/dtale/test_views.py | 14 ++ yarn.lock | 159 ++++++++++++++++++ 19 files changed, 520 insertions(+), 34 deletions(-) create mode 100644 static/__tests__/popups/WordcloudBody-test.jsx create mode 100644 static/popups/charts/WordcloudBody.jsx diff --git a/dtale/views.py b/dtale/views.py index 7b727e38e..8ab3a481b 100644 --- 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 index 9bed0e167..e79c0569b 100644 --- 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 index bd81ff558..30e98f271 100644 --- 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 index 8f76e4543..56330f089 100644 --- 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 index 93af483dc..905e5f96c 100644 --- 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(, { + const props = { + chartData: _.assign({}, chartData, { query: "no-date" }), + dataId: "1", + onClose: _.noop, + propagateState: _.noop, + }; + const result = mount(, { 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 new file mode 100644 index 000000000..8f3ce8d43 --- /dev/null +++ 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(, { + 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(, { + 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 index 9ce0a0848..264d8aa4b 100644 --- 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(, { + 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 index 9f5c61dd4..d97d23a1f 100644 --- 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 index 4fa46ecfe..8cc578443 100644 --- 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); diff --git a/static/dtale/DataViewerMenu.jsx b/static/dtale/DataViewerMenu.jsx index ab04c7361..38642aea1 100644 --- a/static/dtale/DataViewerMenu.jsx +++ b/static/dtale/DataViewerMenu.jsx @@ -9,7 +9,11 @@ import { lockCols, moveToFront, unlockCols, updateSort } from "./dataViewerMenuU import { SORT_PROPS, toggleHeatMap } from "./gridUtils"; function open(path, dataId, height = 450, width = 500) { - window.open(`${path}/${dataId}`, "_blank", `titlebar=1,location=1,status=1,width=${width},height=${height}`); + let fullPath = path; + if (dataId) { + fullPath = `${fullPath}/${dataId}`; + } + window.open(fullPath, "_blank", `titlebar=1,location=1,status=1,width=${width},height=${height}`); } class ReactDataViewerMenu extends React.Component { @@ -123,7 +127,7 @@ class ReactDataViewerMenu extends React.Component {
  • - @@ -200,6 +204,16 @@ class ReactDataViewerMenu extends React.Component {
    • + +
  • + + + +
    • +
  • diff --git a/static/dtale/gridUtils.jsx b/static/dtale/gridUtils.jsx index e39c21024..b7d539d18 100644 --- a/static/dtale/gridUtils.jsx +++ b/static/dtale/gridUtils.jsx @@ -97,13 +97,14 @@ function getRanges(array) { return ranges; } -function calcColWidth({ name, dtype }, { data, rowCount }) { +function calcColWidth({ name, dtype }, { data, rowCount, sortInfo }) { let w = DEFAULT_COL_WIDTH; if (name === IDX) { w = measureText(rowCount - 1 + ""); w = w < DEFAULT_COL_WIDTH ? DEFAULT_COL_WIDTH : w; } else { - const headerWidth = measureText(name); + const sortDir = (_.find(sortInfo, ([col, _dir]) => col === name) || [null, null])[1]; + const headerWidth = measureText(name) + (_.includes(["ASC", "DESC"], sortDir) ? 10 : 0); switch (findColType((dtype || "").toLowerCase())) { case "date": case "int": diff --git a/static/popups/Correlations.jsx b/static/popups/Correlations.jsx index 1841c8d1f..f651b7b4e 100644 --- a/static/popups/Correlations.jsx +++ b/static/popups/Correlations.jsx @@ -29,6 +29,7 @@ function buildState() { selectedCols: [], tsUrl: null, selectedDate: null, + scatterUrl: null, }; } @@ -111,20 +112,25 @@ class ReactCorrelations extends React.Component { } buildScatter(selectedCols, date = null) { - corrUtils.toggleBouncer(); const params = { selectedCols, query: this.props.chartData.query }; if (date) { params.dateCol = this.state.selectedDate; params.date = date; } const path = `${BASE_SCATTER_URL}/${this.props.dataId}`; - fetchJson(buildURL(path, params, ["selectedCols", "query", "date", "dateCol"]), fetchedChartData => { + const scatterUrl = buildURL(path, params, ["selectedCols", "query", "date", "dateCol"]); + if (this.state.scatterUrl === scatterUrl) { + return; + } + corrUtils.toggleBouncer(); + fetchJson(scatterUrl, fetchedChartData => { corrUtils.toggleBouncer(); const newState = { selectedCols, stats: fetchedChartData.stats, date, scatterError: null, + scatterUrl, }; if (fetchedChartData.error) { newState.scatterError = ; diff --git a/static/popups/Describe.jsx b/static/popups/Describe.jsx index 33c18be2c..0fe3b38ca 100644 --- a/static/popups/Describe.jsx +++ b/static/popups/Describe.jsx @@ -71,18 +71,18 @@ class ReactDescribe extends React.Component { } renderUniques() { - const uniques = _.get(this.state, "details.uniques"); - if (_.isEmpty(uniques)) { + const uniques = _.get(this.state, "details.uniques") || {}; + if (_.isEmpty(uniques.data)) { return null; } return (
    - Unique Values: + {`Unique Values${uniques.top ? " (top 100 most common)" : ""}:`}
    - {_.join(_.sortBy(uniques), ", ")} + {_.join(uniques.top ? uniques.data : _.sortBy(uniques.data), ", ")}
    ); @@ -147,7 +147,7 @@ class ReactDescribe extends React.Component { ,
    -
    +
      {_.map(details.describe, (v, k) => (
    • @@ -159,8 +159,10 @@ class ReactDescribe extends React.Component { ))}
    -
    - +
    +
    + +
    , this.renderUniques(), diff --git a/static/popups/charts/Charts.jsx b/static/popups/charts/Charts.jsx index c1de28428..f432fb017 100644 --- a/static/popups/charts/Charts.jsx +++ b/static/popups/charts/Charts.jsx @@ -28,6 +28,7 @@ const AGGREGATIONS = [ { value: "prod", label: "Product of All Items" }, { value: "sum", label: "Sum" }, ]; +const CHART_TYPES = ["line", "bar", "stacked", "pie", "wordcloud"]; function generateChartState({ group, x, y, query, aggregation }, { dataId }) { if (_.isNull(x) || _.isNull(y)) { @@ -220,7 +221,7 @@ class ReactCharts extends React.Component {