[SIP-5] Refactor Paired t-test (apache#5762)

* extract TTestTable into another file. * Move into directory. * update proptypes
kristw · Sep 6, 2018 · 68e7794 · 68e7794
1 parent b284788
commit 68e7794
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 66 deletions.
diff --git a/...ssets/src/visualizations/paired_ttest.css → ...isualizations/PairedTTest/PairedTTest.css b/...ssets/src/visualizations/paired_ttest.css → ...isualizations/PairedTTest/PairedTTest.css
@@ -1,5 +1,5 @@
 .paired_ttest .scrollbar-container {
-  overflow: scroll;
+  overflow: auto;
 }
 
 .paired-ttest-table .scrollbar-content {
@@ -8,6 +8,10 @@
   margin-bottom: 0;
 }
 
+.paired-ttest-table table {
+  margin-bottom: 0;
+}
+
 .paired-ttest-table h1 {
   margin-left: 5px;
 }
@@ -61,7 +65,3 @@
   position: absolute;
   right: 10px;
 }
-
-.paired-ttest-table table {
-  margin-bottom: 0;
-}
diff --git a/superset/assets/src/visualizations/PairedTTest/PairedTTest.jsx b/superset/assets/src/visualizations/PairedTTest/PairedTTest.jsx
@@ -0,0 +1,85 @@
+
+import PropTypes from 'prop-types';
+import ReactDOM from 'react-dom';
+import React from 'react';
+import TTestTable, { dataPropType } from './TTestTable';
+import './PairedTTest.css';
+
+const propTypes = {
+  className: PropTypes.string,
+  metrics: PropTypes.arrayOf(PropTypes.string).isRequired,
+  groups: PropTypes.arrayOf(PropTypes.string).isRequired,
+  data: PropTypes.objectOf(dataPropType).isRequired,
+  alpha: PropTypes.number,
+  liftValPrec: PropTypes.number,
+  pValPrec: PropTypes.number,
+};
+
+const defaultProps = {
+  className: '',
+  alpha: 0.05,
+  liftValPrec: 4,
+  pValPrec: 6,
+};
+
+class PairedTTest extends React.PureComponent {
+  render() {
+    const {
+      className,
+      metrics,
+      groups,
+      data,
+      alpha,
+      pValPrec,
+      liftValPrec,
+    } = this.props;
+    return (
+      <div className={`paired-ttest-table scrollbar-container ${className}`}>
+        <div className="scrollbar-content">
+          {metrics.map((metric, i) => (
+            <TTestTable
+              key={i}
+              metric={metric}
+              groups={groups}
+              data={data[metric]}
+              alpha={alpha}
+              pValPrec={Math.min(pValPrec, 32)}
+              liftValPrec={Math.min(liftValPrec, 32)}
+            />
+          ))}
+        </div>
+      </div>
+    );
+  }
+}
+
+PairedTTest.propTypes = propTypes;
+PairedTTest.defaultProps = defaultProps;
+
+function adaptor(slice, payload) {
+  const { formData, selector } = slice;
+  const element = document.querySelector(selector);
+  const {
+    groupby: groups,
+    metrics,
+    liftvalue_precision: liftValPrec,
+    pvalue_precision: pValPrec,
+    significance_level: alpha,
+  } = formData;
+
+  console.log('groups', groups, payload.data);
+
+  ReactDOM.render(
+    <PairedTTest
+      metrics={metrics}
+      groups={groups}
+      data={payload.data}
+      alpha={alpha}
+      pValPrec={parseInt(pValPrec, 10)}
+      liftValPrec={parseInt(liftValPrec, 10)}
+    />,
+    element,
+  );
+}
+
+export default adaptor;
diff --git a/...ssets/src/visualizations/paired_ttest.jsx → ...visualizations/PairedTTest/TTestTable.jsx b/...ssets/src/visualizations/paired_ttest.jsx → ...visualizations/PairedTTest/TTestTable.jsx
@@ -1,15 +1,32 @@
-import d3 from 'd3';
 import dist from 'distributions';
-
 import React from 'react';
 import { Table, Tr, Td, Thead, Th } from 'reactable';
-import ReactDOM from 'react-dom';
 import PropTypes from 'prop-types';
 
-import './paired_ttest.css';
+export const dataPropType = PropTypes.arrayOf(PropTypes.shape({
+  group: PropTypes.arrayOf(PropTypes.string),
+  values: PropTypes.arrayOf(PropTypes.shape({
+    x: PropTypes.number,
+    y: PropTypes.number,
+  })),
+}));
 
-class TTestTable extends React.Component {
+const propTypes = {
+  metric: PropTypes.string.isRequired,
+  groups: PropTypes.arrayOf(PropTypes.string).isRequired,
+  data: dataPropType.isRequired,
+  alpha: PropTypes.number,
+  liftValPrec: PropTypes.number,
+  pValPrec: PropTypes.number,
+};
+
+const defaultProps = {
+  alpha: 0.05,
+  liftValPrec: 4,
+  pValPrec: 6,
+};
 
+class TTestTable extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
@@ -29,7 +46,7 @@ class TTestTable extends React.Component {
       return 'control';
     }
     const liftVal = this.state.liftValues[row];
-    if (isNaN(liftVal) || !isFinite(liftVal)) {
+    if (Number.isNaN(liftVal) || !Number.isFinite(liftVal)) {
       return 'invalid'; // infinite or NaN values
     }
     return liftVal >= 0 ? 'true' : 'false'; // green on true, red on false
@@ -40,7 +57,7 @@ class TTestTable extends React.Component {
       return 'control';
     }
     const pVal = this.state.pValues[row];
-    if (isNaN(pVal) || !isFinite(pVal)) {
+    if (Number.isNaN(pVal) || !Number.isFinite(pVal)) {
       return 'invalid';
     }
     return ''; // p-values won't normally be colored
@@ -221,57 +238,7 @@ class TTestTable extends React.Component {
   }
 }
 
-TTestTable.propTypes = {
-  metric: PropTypes.string.isRequired,
-  groups: PropTypes.array.isRequired,
-  data: PropTypes.array.isRequired,
-  alpha: PropTypes.number.isRequired,
-  liftValPrec: PropTypes.number.isRequired,
-  pValPrec: PropTypes.number.isRequired,
-};
-TTestTable.defaultProps = {
-  metric: '',
-  groups: [],
-  data: [],
-  alpha: 0.05,
-  liftValPrec: 4,
-  pValPrec: 6,
-};
-
-function pairedTTestVis(slice, payload) {
-  const div = d3.select(slice.selector);
-  const container = slice.container;
-  const height = slice.container.height();
-  const fd = slice.formData;
-  const data = payload.data;
-  const alpha = fd.significance_level;
-  const pValPrec = fd.pvalue_precision;
-  const liftValPrec = fd.liftvalue_precision;
-  const tables = fd.metrics.map((metric, i) => ( // create a table for each metric
-    <TTestTable
-      key={i}
-      metric={metric}
-      groups={fd.groupby}
-      data={data[metric]}
-      alpha={alpha}
-      pValPrec={pValPrec > 32 ? 32 : pValPrec}
-      liftValPrec={liftValPrec > 32 ? 32 : liftValPrec}
-    />
-  ));
-  div.html('');
-  ReactDOM.render(
-    <div className="row">
-      <div className="col-sm-12">
-        <div className="paired-ttest-table scrollbar-container">
-          <div className="scrollbar-content">
-            {tables}
-          </div>
-        </div>
-      </div>
-    </div>,
-    div.node(),
-  );
-  container.find('.scrollbar-container').css('max-height', height);
-}
+TTestTable.propTypes = propTypes;
+TTestTable.defaultProps = defaultProps;
 
-module.exports = pairedTTestVis;
+export default TTestTable;
diff --git a/superset/assets/src/visualizations/index.js b/superset/assets/src/visualizations/index.js
@@ -113,7 +113,7 @@ const vizMap = {
   [VIZ_TYPES.event_flow]: () =>
     loadVis(import(/* webpackChunkName: "EventFlow" */ './EventFlow.jsx')),
   [VIZ_TYPES.paired_ttest]: () =>
-    loadVis(import(/* webpackChunkName: "paired_ttest" */ './paired_ttest.jsx')),
+    loadVis(import(/* webpackChunkName: "paired_ttest" */ './PairedTTest/PairedTTest.jsx')),
   [VIZ_TYPES.partition]: () =>
     loadVis(import(/* webpackChunkName: "partition" */ './partition.js')),
   [VIZ_TYPES.deck_scatter]: () =>