rusackas commented on code in PR #39452:
URL: https://github.com/apache/superset/pull/39452#discussion_r3530947482
##########
superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx:
##########
@@ -32,279 +32,310 @@ export interface DataEntry {
}
interface TTestTableProps {
- alpha: number;
+ alpha?: number;
data: DataEntry[];
groups: string[];
- liftValPrec: number;
+ liftValPrec?: number;
metric: string;
- pValPrec: number;
+ pValPrec?: number;
}
-interface TTestTableState {
- control: number;
- liftValues: (string | number)[];
- pValues: (string | number)[];
-}
+function TTestTable({
+ alpha = 0.05,
+ data,
+ groups,
+ liftValPrec = 4,
+ metric,
+ pValPrec = 6,
+}: TTestTableProps) {
+ const [control, setControl] = useState(0);
+ const [liftValues, setLiftValues] = useState<(string | number)[]>([]);
+ const [pValues, setPValues] = useState<(string | number)[]>([]);
-const defaultProps = {
- alpha: 0.05,
- liftValPrec: 4,
- pValPrec: 6,
-};
+ const computeLift = useCallback(
+ (values: DataPointValue[], controlValues: DataPointValue[]): string => {
+ // Compute the lift value between two time series
+ let sumValues = 0;
+ let sumControl = 0;
+ values.forEach((value, i) => {
+ sumValues += value.y;
+ sumControl += controlValues[i].y;
+ });
-class TTestTable extends Component<TTestTableProps, TTestTableState> {
- static defaultProps = defaultProps;
+ if (sumControl === 0) return 'NaN';
+ return (((sumValues - sumControl) / sumControl) * 100).toFixed(
+ liftValPrec,
+ );
+ },
+ [liftValPrec],
+ );
- constructor(props: TTestTableProps) {
- super(props);
- this.state = {
- control: 0,
- liftValues: [],
- pValues: [],
- };
- }
+ const computePValue = useCallback(
+ (
+ values: DataPointValue[],
+ controlValues: DataPointValue[],
+ ): string | number => {
+ // Compute the p-value from Student's t-test
+ // between two time series
+ let diffSum = 0;
+ let diffSqSum = 0;
+ let finiteCount = 0;
+ values.forEach((value, i) => {
+ const diff = controlValues[i].y - value.y;
+ /* eslint-disable-next-line */
+ if (isFinite(diff)) {
+ finiteCount += 1;
+ diffSum += diff;
+ diffSqSum += diff * diff;
+ }
+ });
+ const tvalue = -Math.abs(
+ diffSum *
+ Math.sqrt(
+ (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
+ ),
+ );
+ try {
+ return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
+ pValPrec,
+ ); // two-sided test
+ } catch (error) {
+ return NaN;
+ }
+ },
+ [pValPrec],
+ );
- componentDidMount() {
- const { control } = this.state;
- this.computeTTest(control); // initially populate table
- }
+ const computeTTest = useCallback(
+ (controlIndex: number) => {
+ // Compute lift and p-values for each row
+ // against the selected control
+ const newPValues: (string | number)[] = [];
+ const newLiftValues: (string | number)[] = [];
+ if (!data) {
+ return;
+ }
+ for (let i = 0; i < data.length; i += 1) {
+ if (i === controlIndex) {
+ newPValues.push('control');
+ newLiftValues.push('control');
+ } else {
+ newPValues.push(
+ computePValue(data[i].values, data[controlIndex].values),
+ );
+ newLiftValues.push(
+ computeLift(data[i].values, data[controlIndex].values),
+ );
+ }
+ }
+ setControl(controlIndex);
+ setLiftValues(newLiftValues);
+ setPValues(newPValues);
+ },
+ [data, computeLift, computePValue],
+ );
- getLiftStatus(row: number): string {
- const { control, liftValues } = this.state;
- // Get a css class name for coloring
- if (row === control) {
- return 'control';
- }
- const liftVal = liftValues[row];
- if (Number.isNaN(liftVal) || !Number.isFinite(liftVal)) {
- return 'invalid'; // infinite or NaN values
+ // Recompute table when data or control row changes, keeping control index
in range
+ useEffect(() => {
+ if (!data || data.length === 0) {
+ setControl(0);
+ setLiftValues([]);
+ setPValues([]);
+ return;
}
- return Number(liftVal) >= 0 ? 'true' : 'false'; // green on true, red on
false
- }
-
- getPValueStatus(row: number): string {
- const { control, pValues } = this.state;
- if (row === control) {
- return 'control';
- }
- const pVal = pValues[row];
- if (Number.isNaN(pVal) || !Number.isFinite(pVal)) {
- return 'invalid';
+ const safeControlIndex = Math.min(control, data.length - 1);
+ if (safeControlIndex !== control) {
+ setControl(safeControlIndex);
+ computeTTest(safeControlIndex);
+ } else {
+ computeTTest(control);
}
+ }, [computeTTest, control, data]);
- return ''; // p-values won't normally be colored
- }
+ const getLiftStatus = useCallback(
+ (row: number): string => {
+ // Get a css class name for coloring
+ if (row === control) {
+ return 'control';
+ }
+ const liftVal = liftValues[row];
+ const numericLiftVal = Number(liftVal);
+ if (Number.isNaN(numericLiftVal) || !Number.isFinite(numericLiftVal)) {
+ return 'invalid'; // infinite or NaN values
+ }
- getSignificance(row: number): string | boolean {
- const { control, pValues } = this.state;
- const { alpha } = this.props;
- // Color significant as green, else red
- if (row === control) {
- return 'control';
- }
+ return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on
false
+ },
+ [control, liftValues],
+ );
- // p-values significant below set threshold
- return Number(pValues[row]) <= alpha;
- }
+ const getPValueStatus = useCallback(
+ (row: number): string => {
+ if (row === control) {
+ return 'control';
+ }
+ const pVal = pValues[row];
+ const numericPVal = Number(pVal);
+ if (Number.isNaN(numericPVal) || !Number.isFinite(numericPVal)) {
+ return 'invalid';
+ }
- computeLift(values: DataPointValue[], control: DataPointValue[]): string {
- const { liftValPrec } = this.props;
- // Compute the lift value between two time series
- let sumValues = 0;
- let sumControl = 0;
- values.forEach((value, i) => {
- sumValues += value.y;
- sumControl += control[i].y;
- });
+ return ''; // p-values won't normally be colored
+ },
+ [control, pValues],
+ );
- return (((sumValues - sumControl) / sumControl) *
100).toFixed(liftValPrec);
- }
-
- computePValue(
- values: DataPointValue[],
- control: DataPointValue[],
- ): string | number {
- const { pValPrec } = this.props;
- // Compute the p-value from Student's t-test
- // between two time series
- let diffSum = 0;
- let diffSqSum = 0;
- let finiteCount = 0;
- values.forEach((value, i) => {
- const diff = control[i].y - value.y;
- /* eslint-disable-next-line */
- if (isFinite(diff)) {
- finiteCount += 1;
- diffSum += diff;
- diffSqSum += diff * diff;
+ const getSignificance = useCallback(
+ (row: number): string | boolean => {
+ // Color significant as green, else red
+ if (row === control) {
+ return 'control';
}
- });
- const tvalue = -Math.abs(
- diffSum *
- Math.sqrt(
- (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
- ),
- );
- try {
- return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
- pValPrec,
- ); // two-sided test
- } catch (error) {
- return NaN;
- }
- }
- computeTTest(control: number) {
- // Compute lift and p-values for each row
- // against the selected control
- const { data } = this.props;
- const pValues: (string | number)[] = [];
- const liftValues: (string | number)[] = [];
- if (!data) {
- return;
- }
- for (let i = 0; i < data.length; i += 1) {
- if (i === control) {
- pValues.push('control');
- liftValues.push('control');
- } else {
- pValues.push(this.computePValue(data[i].values, data[control].values));
- liftValues.push(this.computeLift(data[i].values,
data[control].values));
- }
- }
- this.setState({ control, liftValues, pValues });
- }
+ // p-values significant below set threshold
+ return Number(pValues[row]) <= alpha;
+ },
+ [control, pValues, alpha],
+ );
- render() {
- const { data, metric, groups } = this.props;
- const { control, liftValues, pValues } = this.state;
+ const handleRowClick = useCallback(
+ (rowIndex: number) => {
+ computeTTest(rowIndex);
+ },
+ [computeTTest],
+ );
Review Comment:
Good catch, the effect was recomputing after every row click. It reads the
control index via a ref so a click only computes once.
##########
superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx:
##########
@@ -32,279 +32,310 @@ export interface DataEntry {
}
interface TTestTableProps {
- alpha: number;
+ alpha?: number;
data: DataEntry[];
groups: string[];
- liftValPrec: number;
+ liftValPrec?: number;
metric: string;
- pValPrec: number;
+ pValPrec?: number;
}
-interface TTestTableState {
- control: number;
- liftValues: (string | number)[];
- pValues: (string | number)[];
-}
+function TTestTable({
+ alpha = 0.05,
+ data,
+ groups,
+ liftValPrec = 4,
+ metric,
+ pValPrec = 6,
+}: TTestTableProps) {
+ const [control, setControl] = useState(0);
+ const [liftValues, setLiftValues] = useState<(string | number)[]>([]);
+ const [pValues, setPValues] = useState<(string | number)[]>([]);
-const defaultProps = {
- alpha: 0.05,
- liftValPrec: 4,
- pValPrec: 6,
-};
+ const computeLift = useCallback(
+ (values: DataPointValue[], controlValues: DataPointValue[]): string => {
+ // Compute the lift value between two time series
+ let sumValues = 0;
+ let sumControl = 0;
+ values.forEach((value, i) => {
+ sumValues += value.y;
+ sumControl += controlValues[i].y;
+ });
-class TTestTable extends Component<TTestTableProps, TTestTableState> {
- static defaultProps = defaultProps;
+ if (sumControl === 0) return 'NaN';
+ return (((sumValues - sumControl) / sumControl) * 100).toFixed(
+ liftValPrec,
+ );
+ },
+ [liftValPrec],
+ );
- constructor(props: TTestTableProps) {
- super(props);
- this.state = {
- control: 0,
- liftValues: [],
- pValues: [],
- };
- }
+ const computePValue = useCallback(
+ (
+ values: DataPointValue[],
+ controlValues: DataPointValue[],
+ ): string | number => {
+ // Compute the p-value from Student's t-test
+ // between two time series
+ let diffSum = 0;
+ let diffSqSum = 0;
+ let finiteCount = 0;
+ values.forEach((value, i) => {
+ const diff = controlValues[i].y - value.y;
+ /* eslint-disable-next-line */
+ if (isFinite(diff)) {
+ finiteCount += 1;
+ diffSum += diff;
+ diffSqSum += diff * diff;
+ }
+ });
+ const tvalue = -Math.abs(
+ diffSum *
+ Math.sqrt(
+ (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
+ ),
+ );
+ try {
+ return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
+ pValPrec,
+ ); // two-sided test
+ } catch (error) {
+ return NaN;
+ }
+ },
+ [pValPrec],
+ );
- componentDidMount() {
- const { control } = this.state;
- this.computeTTest(control); // initially populate table
- }
+ const computeTTest = useCallback(
+ (controlIndex: number) => {
+ // Compute lift and p-values for each row
+ // against the selected control
+ const newPValues: (string | number)[] = [];
+ const newLiftValues: (string | number)[] = [];
+ if (!data) {
+ return;
+ }
+ for (let i = 0; i < data.length; i += 1) {
+ if (i === controlIndex) {
+ newPValues.push('control');
+ newLiftValues.push('control');
+ } else {
+ newPValues.push(
+ computePValue(data[i].values, data[controlIndex].values),
+ );
+ newLiftValues.push(
+ computeLift(data[i].values, data[controlIndex].values),
+ );
+ }
+ }
+ setControl(controlIndex);
+ setLiftValues(newLiftValues);
+ setPValues(newPValues);
+ },
+ [data, computeLift, computePValue],
+ );
- getLiftStatus(row: number): string {
- const { control, liftValues } = this.state;
- // Get a css class name for coloring
- if (row === control) {
- return 'control';
- }
- const liftVal = liftValues[row];
- if (Number.isNaN(liftVal) || !Number.isFinite(liftVal)) {
- return 'invalid'; // infinite or NaN values
+ // Recompute table when data or control row changes, keeping control index
in range
+ useEffect(() => {
+ if (!data || data.length === 0) {
+ setControl(0);
+ setLiftValues([]);
+ setPValues([]);
+ return;
}
- return Number(liftVal) >= 0 ? 'true' : 'false'; // green on true, red on
false
- }
-
- getPValueStatus(row: number): string {
- const { control, pValues } = this.state;
- if (row === control) {
- return 'control';
- }
- const pVal = pValues[row];
- if (Number.isNaN(pVal) || !Number.isFinite(pVal)) {
- return 'invalid';
+ const safeControlIndex = Math.min(control, data.length - 1);
+ if (safeControlIndex !== control) {
+ setControl(safeControlIndex);
+ computeTTest(safeControlIndex);
+ } else {
+ computeTTest(control);
}
+ }, [computeTTest, control, data]);
- return ''; // p-values won't normally be colored
- }
+ const getLiftStatus = useCallback(
+ (row: number): string => {
+ // Get a css class name for coloring
+ if (row === control) {
+ return 'control';
+ }
+ const liftVal = liftValues[row];
+ const numericLiftVal = Number(liftVal);
+ if (Number.isNaN(numericLiftVal) || !Number.isFinite(numericLiftVal)) {
+ return 'invalid'; // infinite or NaN values
+ }
- getSignificance(row: number): string | boolean {
- const { control, pValues } = this.state;
- const { alpha } = this.props;
- // Color significant as green, else red
- if (row === control) {
- return 'control';
- }
+ return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on
false
+ },
+ [control, liftValues],
+ );
- // p-values significant below set threshold
- return Number(pValues[row]) <= alpha;
- }
+ const getPValueStatus = useCallback(
+ (row: number): string => {
+ if (row === control) {
+ return 'control';
+ }
+ const pVal = pValues[row];
+ const numericPVal = Number(pVal);
+ if (Number.isNaN(numericPVal) || !Number.isFinite(numericPVal)) {
+ return 'invalid';
+ }
- computeLift(values: DataPointValue[], control: DataPointValue[]): string {
- const { liftValPrec } = this.props;
- // Compute the lift value between two time series
- let sumValues = 0;
- let sumControl = 0;
- values.forEach((value, i) => {
- sumValues += value.y;
- sumControl += control[i].y;
- });
+ return ''; // p-values won't normally be colored
+ },
+ [control, pValues],
+ );
- return (((sumValues - sumControl) / sumControl) *
100).toFixed(liftValPrec);
- }
-
- computePValue(
- values: DataPointValue[],
- control: DataPointValue[],
- ): string | number {
- const { pValPrec } = this.props;
- // Compute the p-value from Student's t-test
- // between two time series
- let diffSum = 0;
- let diffSqSum = 0;
- let finiteCount = 0;
- values.forEach((value, i) => {
- const diff = control[i].y - value.y;
- /* eslint-disable-next-line */
- if (isFinite(diff)) {
- finiteCount += 1;
- diffSum += diff;
- diffSqSum += diff * diff;
+ const getSignificance = useCallback(
+ (row: number): string | boolean => {
+ // Color significant as green, else red
+ if (row === control) {
+ return 'control';
}
- });
- const tvalue = -Math.abs(
- diffSum *
- Math.sqrt(
- (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
- ),
- );
- try {
- return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
- pValPrec,
- ); // two-sided test
- } catch (error) {
- return NaN;
- }
- }
- computeTTest(control: number) {
- // Compute lift and p-values for each row
- // against the selected control
- const { data } = this.props;
- const pValues: (string | number)[] = [];
- const liftValues: (string | number)[] = [];
- if (!data) {
- return;
- }
- for (let i = 0; i < data.length; i += 1) {
- if (i === control) {
- pValues.push('control');
- liftValues.push('control');
- } else {
- pValues.push(this.computePValue(data[i].values, data[control].values));
- liftValues.push(this.computeLift(data[i].values,
data[control].values));
- }
- }
- this.setState({ control, liftValues, pValues });
- }
+ // p-values significant below set threshold
+ return Number(pValues[row]) <= alpha;
+ },
+ [control, pValues, alpha],
+ );
- render() {
- const { data, metric, groups } = this.props;
- const { control, liftValues, pValues } = this.state;
+ const handleRowClick = useCallback(
+ (rowIndex: number) => {
+ computeTTest(rowIndex);
+ },
+ [computeTTest],
+ );
- if (!Array.isArray(groups) || groups.length === 0) {
- throw new Error('Group by param is required');
- }
+ if (!Array.isArray(groups) || groups.length === 0) {
+ throw new Error('Group by param is required');
+ }
- // Render column header for each group
- const columns = groups.map((group, i) => (
- <Th key={i} column={group}>
- {group}
- </Th>
- ));
- const numGroups = groups.length;
- // Columns for p-value, lift-value, and significance (true/false)
- columns.push(
- <Th key={numGroups + 1} column="pValue">
- p-value
- </Th>,
+ // Render column header for each group
+ const columns = groups.map((group, i) => (
+ <Th key={i} column={group}>
+ {group}
+ </Th>
+ ));
+ const numGroups = groups.length;
+ // Columns for p-value, lift-value, and significance (true/false)
+ columns.push(
+ <Th key={numGroups + 1} column="pValue">
+ p-value
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 2} column="liftValue">
+ Lift %
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 3} column="significant">
+ Significant
+ </Th>,
+ );
+
+ const rows = data.map((entry, i) => {
Review Comment:
Added a guard on the render path so a missing metric key does not blow up
the row `.map`.
##########
superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx:
##########
@@ -32,279 +32,310 @@ export interface DataEntry {
}
interface TTestTableProps {
- alpha: number;
+ alpha?: number;
data: DataEntry[];
groups: string[];
- liftValPrec: number;
+ liftValPrec?: number;
metric: string;
- pValPrec: number;
+ pValPrec?: number;
}
-interface TTestTableState {
- control: number;
- liftValues: (string | number)[];
- pValues: (string | number)[];
-}
+function TTestTable({
+ alpha = 0.05,
+ data,
+ groups,
+ liftValPrec = 4,
+ metric,
+ pValPrec = 6,
+}: TTestTableProps) {
+ const [control, setControl] = useState(0);
+ const [liftValues, setLiftValues] = useState<(string | number)[]>([]);
+ const [pValues, setPValues] = useState<(string | number)[]>([]);
-const defaultProps = {
- alpha: 0.05,
- liftValPrec: 4,
- pValPrec: 6,
-};
+ const computeLift = useCallback(
+ (values: DataPointValue[], controlValues: DataPointValue[]): string => {
+ // Compute the lift value between two time series
+ let sumValues = 0;
+ let sumControl = 0;
+ values.forEach((value, i) => {
+ sumValues += value.y;
+ sumControl += controlValues[i].y;
+ });
-class TTestTable extends Component<TTestTableProps, TTestTableState> {
- static defaultProps = defaultProps;
+ if (sumControl === 0) return 'NaN';
+ return (((sumValues - sumControl) / sumControl) * 100).toFixed(
+ liftValPrec,
+ );
+ },
+ [liftValPrec],
+ );
- constructor(props: TTestTableProps) {
- super(props);
- this.state = {
- control: 0,
- liftValues: [],
- pValues: [],
- };
- }
+ const computePValue = useCallback(
+ (
+ values: DataPointValue[],
+ controlValues: DataPointValue[],
+ ): string | number => {
+ // Compute the p-value from Student's t-test
+ // between two time series
+ let diffSum = 0;
+ let diffSqSum = 0;
+ let finiteCount = 0;
+ values.forEach((value, i) => {
+ const diff = controlValues[i].y - value.y;
+ /* eslint-disable-next-line */
+ if (isFinite(diff)) {
+ finiteCount += 1;
+ diffSum += diff;
+ diffSqSum += diff * diff;
+ }
+ });
+ const tvalue = -Math.abs(
+ diffSum *
+ Math.sqrt(
+ (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
+ ),
+ );
+ try {
+ return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
+ pValPrec,
+ ); // two-sided test
+ } catch (error) {
+ return NaN;
+ }
+ },
+ [pValPrec],
+ );
- componentDidMount() {
- const { control } = this.state;
- this.computeTTest(control); // initially populate table
- }
+ const computeTTest = useCallback(
+ (controlIndex: number) => {
+ // Compute lift and p-values for each row
+ // against the selected control
+ const newPValues: (string | number)[] = [];
+ const newLiftValues: (string | number)[] = [];
+ if (!data) {
+ return;
+ }
+ for (let i = 0; i < data.length; i += 1) {
+ if (i === controlIndex) {
+ newPValues.push('control');
+ newLiftValues.push('control');
+ } else {
+ newPValues.push(
+ computePValue(data[i].values, data[controlIndex].values),
+ );
+ newLiftValues.push(
+ computeLift(data[i].values, data[controlIndex].values),
+ );
+ }
+ }
+ setControl(controlIndex);
+ setLiftValues(newLiftValues);
+ setPValues(newPValues);
+ },
+ [data, computeLift, computePValue],
+ );
- getLiftStatus(row: number): string {
- const { control, liftValues } = this.state;
- // Get a css class name for coloring
- if (row === control) {
- return 'control';
- }
- const liftVal = liftValues[row];
- if (Number.isNaN(liftVal) || !Number.isFinite(liftVal)) {
- return 'invalid'; // infinite or NaN values
+ // Recompute table when data or control row changes, keeping control index
in range
+ useEffect(() => {
+ if (!data || data.length === 0) {
+ setControl(0);
+ setLiftValues([]);
+ setPValues([]);
+ return;
}
- return Number(liftVal) >= 0 ? 'true' : 'false'; // green on true, red on
false
- }
-
- getPValueStatus(row: number): string {
- const { control, pValues } = this.state;
- if (row === control) {
- return 'control';
- }
- const pVal = pValues[row];
- if (Number.isNaN(pVal) || !Number.isFinite(pVal)) {
- return 'invalid';
+ const safeControlIndex = Math.min(control, data.length - 1);
+ if (safeControlIndex !== control) {
+ setControl(safeControlIndex);
+ computeTTest(safeControlIndex);
+ } else {
+ computeTTest(control);
}
+ }, [computeTTest, control, data]);
- return ''; // p-values won't normally be colored
- }
+ const getLiftStatus = useCallback(
+ (row: number): string => {
+ // Get a css class name for coloring
+ if (row === control) {
+ return 'control';
+ }
+ const liftVal = liftValues[row];
+ const numericLiftVal = Number(liftVal);
+ if (Number.isNaN(numericLiftVal) || !Number.isFinite(numericLiftVal)) {
+ return 'invalid'; // infinite or NaN values
+ }
- getSignificance(row: number): string | boolean {
- const { control, pValues } = this.state;
- const { alpha } = this.props;
- // Color significant as green, else red
- if (row === control) {
- return 'control';
- }
+ return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on
false
+ },
+ [control, liftValues],
+ );
- // p-values significant below set threshold
- return Number(pValues[row]) <= alpha;
- }
+ const getPValueStatus = useCallback(
+ (row: number): string => {
+ if (row === control) {
+ return 'control';
+ }
+ const pVal = pValues[row];
+ const numericPVal = Number(pVal);
+ if (Number.isNaN(numericPVal) || !Number.isFinite(numericPVal)) {
+ return 'invalid';
+ }
- computeLift(values: DataPointValue[], control: DataPointValue[]): string {
- const { liftValPrec } = this.props;
- // Compute the lift value between two time series
- let sumValues = 0;
- let sumControl = 0;
- values.forEach((value, i) => {
- sumValues += value.y;
- sumControl += control[i].y;
- });
+ return ''; // p-values won't normally be colored
+ },
+ [control, pValues],
+ );
- return (((sumValues - sumControl) / sumControl) *
100).toFixed(liftValPrec);
- }
-
- computePValue(
- values: DataPointValue[],
- control: DataPointValue[],
- ): string | number {
- const { pValPrec } = this.props;
- // Compute the p-value from Student's t-test
- // between two time series
- let diffSum = 0;
- let diffSqSum = 0;
- let finiteCount = 0;
- values.forEach((value, i) => {
- const diff = control[i].y - value.y;
- /* eslint-disable-next-line */
- if (isFinite(diff)) {
- finiteCount += 1;
- diffSum += diff;
- diffSqSum += diff * diff;
+ const getSignificance = useCallback(
+ (row: number): string | boolean => {
+ // Color significant as green, else red
+ if (row === control) {
+ return 'control';
}
- });
- const tvalue = -Math.abs(
- diffSum *
- Math.sqrt(
- (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
- ),
- );
- try {
- return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
- pValPrec,
- ); // two-sided test
- } catch (error) {
- return NaN;
- }
- }
- computeTTest(control: number) {
- // Compute lift and p-values for each row
- // against the selected control
- const { data } = this.props;
- const pValues: (string | number)[] = [];
- const liftValues: (string | number)[] = [];
- if (!data) {
- return;
- }
- for (let i = 0; i < data.length; i += 1) {
- if (i === control) {
- pValues.push('control');
- liftValues.push('control');
- } else {
- pValues.push(this.computePValue(data[i].values, data[control].values));
- liftValues.push(this.computeLift(data[i].values,
data[control].values));
- }
- }
- this.setState({ control, liftValues, pValues });
- }
+ // p-values significant below set threshold
+ return Number(pValues[row]) <= alpha;
+ },
+ [control, pValues, alpha],
+ );
- render() {
- const { data, metric, groups } = this.props;
- const { control, liftValues, pValues } = this.state;
+ const handleRowClick = useCallback(
+ (rowIndex: number) => {
+ computeTTest(rowIndex);
+ },
+ [computeTTest],
+ );
- if (!Array.isArray(groups) || groups.length === 0) {
- throw new Error('Group by param is required');
- }
+ if (!Array.isArray(groups) || groups.length === 0) {
+ throw new Error('Group by param is required');
+ }
- // Render column header for each group
- const columns = groups.map((group, i) => (
- <Th key={i} column={group}>
- {group}
- </Th>
- ));
- const numGroups = groups.length;
- // Columns for p-value, lift-value, and significance (true/false)
- columns.push(
- <Th key={numGroups + 1} column="pValue">
- p-value
- </Th>,
+ // Render column header for each group
+ const columns = groups.map((group, i) => (
+ <Th key={i} column={group}>
+ {group}
+ </Th>
+ ));
+ const numGroups = groups.length;
+ // Columns for p-value, lift-value, and significance (true/false)
+ columns.push(
+ <Th key={numGroups + 1} column="pValue">
+ p-value
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 2} column="liftValue">
+ Lift %
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 3} column="significant">
+ Significant
+ </Th>,
+ );
+
+ const rows = data.map((entry, i) => {
+ const values = groups.map(
+ (
+ group,
+ j, // group names
+ ) => <Td key={j} column={group} data={entry.group[j]} />,
);
- columns.push(
- <Th key={numGroups + 2} column="liftValue">
- Lift %
- </Th>,
+ values.push(
+ <Td
+ key={numGroups + 1}
+ className={getPValueStatus(i)}
+ column="pValue"
+ data={pValues[i]}
+ />,
);
- columns.push(
- <Th key={numGroups + 3} column="significant">
- Significant
- </Th>,
+ values.push(
+ <Td
+ key={numGroups + 2}
+ className={getLiftStatus(i)}
+ column="liftValue"
+ data={liftValues[i]}
+ />,
+ );
+ values.push(
+ <Td
+ key={numGroups + 3}
+ className={getSignificance(i).toString()}
+ column="significant"
+ data={getSignificance(i)}
+ />,
);
- const rows = data.map((entry, i) => {
- const values = groups.map(
- (
- group,
- j, // group names
- ) => <Td key={j} column={group} data={entry.group[j]} />,
- );
- values.push(
- <Td
- key={numGroups + 1}
- className={this.getPValueStatus(i)}
- column="pValue"
- data={pValues[i]}
- />,
- );
- values.push(
- <Td
- key={numGroups + 2}
- className={this.getLiftStatus(i)}
- column="liftValue"
- data={liftValues[i]}
- />,
- );
- values.push(
- <Td
- key={numGroups + 3}
- className={this.getSignificance(i).toString()}
- column="significant"
- data={this.getSignificance(i)}
- />,
- );
- return (
- <Tr
- key={i}
- className={i === control ? 'control' : ''}
- onClick={this.computeTTest.bind(this, i)}
- >
- {values}
- </Tr>
- );
- });
- // When sorted ascending, 'control' will always be at top
- type SortConfigItem =
- | string
- | { column: string; sortFunction: (a: string, b: string) => number };
- const sortConfig: SortConfigItem[] = (groups as SortConfigItem[]).concat([
- {
- column: 'pValue',
- sortFunction: (a: string, b: string) => {
- if (a === 'control') {
- return -1;
- }
- if (b === 'control') {
- return 1;
- }
+ return (
+ <Tr
+ key={i}
+ className={i === control ? 'control' : ''}
+ onClick={() => handleRowClick(i)}
+ >
+ {values}
+ </Tr>
+ );
+ });
- return a > b ? 1 : -1; // p-values ascending
+ // When sorted ascending, 'control' will always be at top
+ type SortConfigItem =
+ string | { column: string; sortFunction: (a: string, b: string) => number
};
+
+ const sortConfig: SortConfigItem[] = useMemo(
+ () =>
+ (groups as SortConfigItem[]).concat([
+ {
+ column: 'pValue',
+ sortFunction: (a: string, b: string) => {
+ if (a === 'control') {
+ return -1;
+ }
+ if (b === 'control') {
+ return 1;
+ }
+
+ return a > b ? 1 : -1; // p-values ascending
Review Comment:
Fair point... the comparators return 0 on equality now.
##########
superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx:
##########
@@ -196,61 +185,80 @@ class SimpleEncoder {
}
}
-class WordCloud extends PureComponent<FullWordCloudProps, WordCloudState> {
- static defaultProps = defaultProps;
-
- isComponentMounted = false;
-
- createEncoder = (encoding?: Partial<WordCloudEncoding>): SimpleEncoder =>
- new SimpleEncoder(encoding ?? {}, {
- color: this.props.theme.colorTextLabel,
- fontFamily: this.props.theme.fontFamily,
- fontSize: 20,
- fontWeight: 'bold',
- text: '',
- });
-
- constructor(props: FullWordCloudProps) {
- super(props);
- this.state = {
- words: [],
- scaleFactor: 1,
- };
- this.setWords = this.setWords.bind(this);
- }
-
- componentDidMount() {
- this.isComponentMounted = true;
- this.update();
- }
-
- componentDidUpdate(prevProps: WordCloudProps) {
- const { data, encoding, width, height, rotation } = this.props;
- if (
- !isEqual(prevProps.data, data) ||
- !isEqual(prevProps.encoding, encoding) ||
- prevProps.width !== width ||
- prevProps.height !== height ||
- prevProps.rotation !== rotation
- ) {
- this.update();
- }
- }
-
- componentWillUnmount() {
- this.isComponentMounted = false;
- }
-
- setWords(words: Word[]) {
- if (this.isComponentMounted) {
- this.setState({ words });
+function WordCloud({
+ data,
+ encoding = {},
+ width,
+ height,
+ rotation = 'flat',
+ sliceId,
+ colorScheme,
+ theme,
+}: FullWordCloudProps) {
+ const [words, setWords] = useState<Word[]>([]);
+ const [scaleFactor] = useState(1);
Review Comment:
This turned out to be behavior dropped in an earlier refactor... the
original code persisted the accepted scale factor via setState. Restored it, so
the viewBox tracks the enlarged canvas.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
