You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/09/06 23:14:29 UTC

[GitHub] williaster closed pull request #5762: [SIP-5] Refactor Paired t-test

williaster closed pull request #5762: [SIP-5] Refactor Paired t-test
URL: https://github.com/apache/incubator-superset/pull/5762
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/src/visualizations/paired_ttest.css b/superset/assets/src/visualizations/PairedTTest/PairedTTest.css
similarity index 98%
rename from superset/assets/src/visualizations/paired_ttest.css
rename to superset/assets/src/visualizations/PairedTTest/PairedTTest.css
index 0a2c1b8d28..95068ca301 100644
--- a/superset/assets/src/visualizations/paired_ttest.css
+++ b/superset/assets/src/visualizations/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
new file mode 100644
index 0000000000..3a26e9d399
--- /dev/null
+++ 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/superset/assets/src/visualizations/paired_ttest.jsx b/superset/assets/src/visualizations/PairedTTest/TTestTable.jsx
similarity index 79%
rename from superset/assets/src/visualizations/paired_ttest.jsx
rename to superset/assets/src/visualizations/PairedTTest/TTestTable.jsx
index e715f02358..06c880be2c 100644
--- a/superset/assets/src/visualizations/paired_ttest.jsx
+++ b/superset/assets/src/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
index 66cff81859..93e680915c 100644
--- 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]: () =>


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org