From c0abd8d3b8f9eb436cccfc9a8d7d490ca2e8bdb6 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Fri, 17 Apr 2026 10:33:34 -0700 Subject: [PATCH] chore(lint): convert legacy plugin chart components to function components Converts HorizonChart, HorizonRow, MapBox, ScatterPlotGlowOverlay, PairedTTest, TTestTable, and WordCloud from class components to function components. Includes updated TTestTable test. Co-Authored-By: Claude Sonnet 4.6 --- .../src/HorizonChart.tsx | 103 ++-- .../src/HorizonRow.tsx | 244 ++++---- .../src/PairedTTest.tsx | 73 ++- .../src/TTestTable.tsx | 535 +++++++++--------- .../test/TTestTable.test.tsx | 285 ++++++++++ .../src/chart/WordCloud.tsx | 246 ++++---- 6 files changed, 880 insertions(+), 606 deletions(-) create mode 100644 superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx diff --git a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx index a95cae279dd..2ef267ef336 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx @@ -16,11 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -/* eslint-disable react/jsx-sort-default-props, react/sort-prop-types */ -import { PureComponent } from 'react'; +import { memo, useMemo } from 'react'; import { extent as d3Extent } from 'd3-array'; import { ensureIsArray } from '@superset-ui/core'; -import { styled } from '@apache-superset/core/theme'; +import { styled } from '@apache-superset/core/ui'; import HorizonRow, { DEFAULT_COLORS } from './HorizonRow'; interface DataValue { @@ -45,18 +44,6 @@ interface HorizonChartProps { offsetX?: number; } -const defaultProps: Partial = { - className: '', - width: 800, - height: 600, - seriesHeight: 20, - bands: Math.floor(DEFAULT_COLORS.length / 2), - colors: DEFAULT_COLORS, - colorScale: 'series', - mode: 'offset', - offsetX: 0, -}; - const StyledDiv = styled.div` ${({ theme }) => ` .superset-legacy-chart-horizon { @@ -80,24 +67,19 @@ const StyledDiv = styled.div` `} `; -class HorizonChart extends PureComponent { - static defaultProps = defaultProps; - - render() { - const { - className, - width, - height, - data, - seriesHeight, - bands, - colors, - colorScale, - mode, - offsetX, - } = this.props; - - let yDomain: [number, number] | undefined; +function HorizonChart({ + className = '', + width = 800, + height = 600, + seriesHeight = 20, + data, + bands = Math.floor(DEFAULT_COLORS.length / 2), + colors = DEFAULT_COLORS, + colorScale = 'series', + mode = 'offset', + offsetX = 0, +}: HorizonChartProps) { + const yDomain = useMemo((): [number, number] | undefined => { if (colorScale === 'overall') { const allValues = data.reduce( (acc, current) => acc.concat(current.values), @@ -106,35 +88,36 @@ class HorizonChart extends PureComponent { const rawExtent = d3Extent(allValues, d => d.y); // Only set yDomain if we have valid min and max values if (rawExtent[0] != null && rawExtent[1] != null) { - yDomain = [rawExtent[0], rawExtent[1]]; + return [rawExtent[0], rawExtent[1]]; } } + return undefined; + }, [colorScale, data]); - return ( - -
- {data.map(row => ( - - ))} -
- - ); - } + return ( + +
+ {data.map(row => ( + + ))} +
+ + ); } -export default HorizonChart; +export default memo(HorizonChart); diff --git a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx index f399d850d7c..5431674b7e9 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx @@ -17,9 +17,7 @@ * under the License. */ /* eslint-disable no-continue, no-bitwise */ -/* eslint-disable react/jsx-sort-default-props */ -/* eslint-disable react/sort-prop-types */ -import { PureComponent } from 'react'; +import { useRef, useEffect, useCallback, memo } from 'react'; import { extent as d3Extent } from 'd3-array'; import { scaleLinear } from 'd3-scale'; @@ -52,162 +50,140 @@ interface HorizonRowProps { yDomain?: [number, number]; } -const defaultProps: Partial = { - className: '', - width: 800, - height: 20, - bands: DEFAULT_COLORS.length >> 1, - colors: DEFAULT_COLORS, - colorScale: 'series', - mode: 'offset', - offsetX: 0, - title: '', - yDomain: undefined, -}; +function HorizonRow({ + className = '', + width = 800, + height = 20, + data: rawData, + bands = DEFAULT_COLORS.length >> 1, + colors = DEFAULT_COLORS, + colorScale = 'series', + mode = 'offset', + offsetX = 0, + title = '', + yDomain, +}: HorizonRowProps) { + const canvasRef = useRef(null); -class HorizonRow extends PureComponent { - static defaultProps = defaultProps; + const drawChart = useCallback(() => { + const canvas = canvasRef.current; + if (!canvas) return; - private canvas: HTMLCanvasElement | null = null; + const data = + colorScale === 'change' && rawData.length > 0 + ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y })) + : rawData; - componentDidMount() { - this.drawChart(); - } + const context = canvas.getContext('2d'); + if (!context) return; + context.imageSmoothingEnabled = false; + context.clearRect(0, 0, width, height); + // Reset transform + context.setTransform(1, 0, 0, 1, 0, 0); + context.translate(0.5, 0.5); - componentDidUpdate() { - this.drawChart(); - } + const step = width / data.length; + // the data frame currently being shown: + const startIndex = Math.floor(Math.max(0, -(offsetX / step))); + const endIndex = Math.floor( + Math.min(data.length, startIndex + width / step), + ); - componentWillUnmount() { - this.canvas = null; - } + // skip drawing if there's no data to be drawn + if (startIndex > data.length) { + return; + } - drawChart() { - if (this.canvas) { - const { - data: rawData, - yDomain, - width = 800, - height = 20, - bands = DEFAULT_COLORS.length >> 1, - colors = DEFAULT_COLORS, - colorScale, - offsetX = 0, - mode, - } = this.props; + // Create y-scale + const [min, max] = + yDomain || (d3Extent(data, d => d.y) as [number, number]); + const y = scaleLinear() + .domain([0, Math.max(-min, max)]) + .range([0, height]); - const data = - colorScale === 'change' - ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y })) - : rawData; + // we are drawing positive & negative bands separately to avoid mutating canvas state + // http://www.html5rocks.com/en/tutorials/canvas/performance/ + let hasNegative = false; + // draw positive bands + let value: number; + let bExtents: number; + for (let b = 0; b < bands; b += 1) { + context.fillStyle = colors[bands + b]; - const context = this.canvas.getContext('2d'); - if (!context) return; - context.imageSmoothingEnabled = false; - context.clearRect(0, 0, width, height); - // Reset transform - context.setTransform(1, 0, 0, 1, 0, 0); - context.translate(0.5, 0.5); + // Adjust the range based on the current band index. + bExtents = (b + 1 - bands) * height; + y.range([bands * height + bExtents, bExtents]); - const step = width / data.length; - // the data frame currently being shown: - const startIndex = Math.floor(Math.max(0, -(offsetX / step))); - const endIndex = Math.floor( - Math.min(data.length, startIndex + width / step), - ); + // only the current data frame is being drawn i.e. what's visible: + for (let i = startIndex; i < endIndex; i += 1) { + value = data[i].y; + if (value <= 0) { + hasNegative = true; + continue; + } + if (value !== undefined) { + context.fillRect( + offsetX + i * step, + y(value)!, + step + 1, + y(0)! - y(value)!, + ); + } + } + } - // skip drawing if there's no data to be drawn - if (startIndex > data.length) { - return; + // draw negative bands + if (hasNegative) { + // mirror the negative bands, by flipping the canvas + if (mode === 'offset') { + context.translate(0, height); + context.scale(1, -1); } - // Create y-scale - const [min, max] = - yDomain || (d3Extent(data, d => d.y) as [number, number]); - const y = scaleLinear() - .domain([0, Math.max(-min, max)]) - .range([0, height]); - - // we are drawing positive & negative bands separately to avoid mutating canvas state - // http://www.html5rocks.com/en/tutorials/canvas/performance/ - let hasNegative = false; - // draw positive bands - let value: number; - let bExtents: number; for (let b = 0; b < bands; b += 1) { - context.fillStyle = colors[bands + b]; + context.fillStyle = colors[bands - b - 1]; // Adjust the range based on the current band index. bExtents = (b + 1 - bands) * height; y.range([bands * height + bExtents, bExtents]); // only the current data frame is being drawn i.e. what's visible: - for (let i = startIndex; i < endIndex; i += 1) { - value = data[i].y; - if (value <= 0) { - hasNegative = true; + for (let ii = startIndex; ii < endIndex; ii += 1) { + value = data[ii].y; + if (value >= 0) { continue; } - if (value !== undefined) { - context.fillRect( - offsetX + i * step, - y(value)!, - step + 1, - y(0)! - y(value)!, - ); - } - } - } - - // draw negative bands - if (hasNegative) { - // mirror the negative bands, by flipping the canvas - if (mode === 'offset') { - context.translate(0, height); - context.scale(1, -1); - } - - for (let b = 0; b < bands; b += 1) { - context.fillStyle = colors[bands - b - 1]; - - // Adjust the range based on the current band index. - bExtents = (b + 1 - bands) * height; - y.range([bands * height + bExtents, bExtents]); - - // only the current data frame is being drawn i.e. what's visible: - for (let ii = startIndex; ii < endIndex; ii += 1) { - value = data[ii].y; - if (value >= 0) { - continue; - } - context.fillRect( - offsetX + ii * step, - y(-value)!, - step + 1, - y(0)! - y(-value)!, - ); - } + context.fillRect( + offsetX + ii * step, + y(-value)!, + step + 1, + y(0)! - y(-value)!, + ); } } } - } + }, [ + rawData, + yDomain, + width, + height, + bands, + colors, + colorScale, + offsetX, + mode, + ]); - render() { - const { className, title, width, height } = this.props; + useEffect(() => { + drawChart(); + }, [drawChart]); - return ( -
- {title} - { - this.canvas = c; - }} - width={width} - height={height} - /> -
- ); - } + return ( +
+ {title} + +
+ ); } -export default HorizonRow; +export default memo(HorizonRow); diff --git a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx index c7a1edfa897..8c2aa161750 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx @@ -17,27 +17,19 @@ * under the License. */ /* eslint-disable react/no-array-index-key */ -import { PureComponent } from 'react'; -import { styled } from '@apache-superset/core/theme'; +import { styled } from '@apache-superset/core/ui'; import TTestTable, { DataEntry } from './TTestTable'; interface PairedTTestProps { - alpha: number; - className: string; + alpha?: number; + className?: string; data: Record; groups: string[]; - liftValPrec: number; + liftValPrec?: number; metrics: string[]; - pValPrec: number; + pValPrec?: number; } -const defaultProps = { - alpha: 0.05, - className: '', - liftValPrec: 4, - pValPrec: 6, -}; - const StyledDiv = styled.div` ${({ theme }) => ` .superset-legacy-chart-paired_ttest .scrollbar-container { @@ -114,35 +106,36 @@ const StyledDiv = styled.div` `} `; -class PairedTTest extends PureComponent { - static defaultProps = defaultProps; - - render() { - const { className, metrics, groups, data, alpha, pValPrec, liftValPrec } = - this.props; - - return ( - -
-
-
- {metrics.map((metric, i) => ( - - ))} -
+function PairedTTest({ + alpha = 0.05, + className = '', + data, + groups, + liftValPrec = 4, + metrics, + pValPrec = 6, +}: PairedTTestProps) { + return ( + +
+
+
+ {metrics.map((metric, i) => ( + + ))}
- - ); - } +
+ + ); } export default PairedTTest; diff --git a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx index 95922fd32de..161d46b5182 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx @@ -18,7 +18,7 @@ */ /* eslint-disable react/no-array-index-key, react/jsx-no-bind */ import dist from 'distributions'; -import { Component } from 'react'; +import { useState, useEffect, useCallback, useMemo } from 'react'; import { Table, Tr, Td, Thead, Th } from 'reactable'; interface DataPointValue { @@ -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 { - 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: [], - }; - } - - componentDidMount() { - const { control } = this.state; - this.computeTTest(control); // initially populate table - } - - 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 - } - - 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'; - } - - return ''; // p-values won't normally be colored - } - - 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'; - } - - // p-values significant below set threshold - return Number(pValues[row]) <= alpha; - } - - 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 (((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 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; } - }); - 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], + ); - 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) { + 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], + ); + + // Recompute table when data or control row changes, keeping control index in range + useEffect(() => { + if (!data || data.length === 0) { + setControl(0); + setLiftValues([]); + setPValues([]); 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)); - } + + const safeControlIndex = Math.min(control, data.length - 1); + if (safeControlIndex !== control) { + setControl(safeControlIndex); + computeTTest(safeControlIndex); + } else { + computeTTest(control); } - this.setState({ control, liftValues, pValues }); + }, [computeTTest, control, data]); + + 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 + } + + return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on false + }, + [control, liftValues], + ); + + 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'; + } + + return ''; // p-values won't normally be colored + }, + [control, pValues], + ); + + const getSignificance = useCallback( + (row: number): string | boolean => { + // Color significant as green, else red + if (row === control) { + return 'control'; + } + + // p-values significant below set threshold + return Number(pValues[row]) <= alpha; + }, + [control, pValues, alpha], + ); + + const handleRowClick = useCallback( + (rowIndex: number) => { + computeTTest(rowIndex); + }, + [computeTTest], + ); + + if (!Array.isArray(groups) || groups.length === 0) { + throw new Error('Group by param is required'); } - render() { - const { data, metric, groups } = this.props; - const { control, liftValues, pValues } = this.state; + // Render column header for each group + const columns = groups.map((group, i) => ( + + {group} + + )); + const numGroups = groups.length; + // Columns for p-value, lift-value, and significance (true/false) + columns.push( + + p-value + , + ); + columns.push( + + Lift % + , + ); + columns.push( + + Significant + , + ); - 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) => ( - - {group} - - )); - const numGroups = groups.length; - // Columns for p-value, lift-value, and significance (true/false) - columns.push( - - p-value - , + const rows = data.map((entry, i) => { + const values = groups.map( + ( + group, + j, // group names + ) => , ); - columns.push( - - Lift % - , + values.push( + , ); - columns.push( - - Significant - , + values.push( + , + ); + values.push( + , ); - const rows = data.map((entry, i) => { - const values = groups.map( - ( - group, - j, // group names - ) => , - ); - values.push( - , - ); - values.push( - , - ); - values.push( - , - ); - - return ( - - {values} - - ); - }); - // 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 a > b ? 1 : -1; // p-values ascending - }, - }, - { - column: 'liftValue', - sortFunction: (a: string, b: string) => { - if (a === 'control') { - return -1; - } - if (b === 'control') { - return 1; - } - - return parseFloat(a) > parseFloat(b) ? -1 : 1; // lift values descending - }, - }, - { - column: 'significant', - sortFunction: (a: string, b: string) => { - if (a === 'control') { - return -1; - } - if (b === 'control') { - return 1; - } - - return a > b ? -1 : 1; // significant values first - }, - }, - ]); return ( -
-

{metric}

- - {columns} - {rows} -
-
+ handleRowClick(i)} + > + {values} + ); - } + }); + + // 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 + }, + }, + { + column: 'liftValue', + sortFunction: (a: string, b: string) => { + if (a === 'control') { + return -1; + } + if (b === 'control') { + return 1; + } + + return parseFloat(a) > parseFloat(b) ? -1 : 1; // lift values descending + }, + }, + { + column: 'significant', + sortFunction: (a: string, b: string) => { + if (a === 'control') { + return -1; + } + if (b === 'control') { + return 1; + } + + return a > b ? -1 : 1; // significant values first + }, + }, + ]), + [groups], + ); + + return ( +
+

{metric}

+ + {columns} + {rows} +
+
+ ); } export default TTestTable; diff --git a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx new file mode 100644 index 00000000000..9011410300f --- /dev/null +++ b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx @@ -0,0 +1,285 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import TTestTable from '../src/TTestTable'; +import type { DataEntry } from '../src/TTestTable'; + +// Mock the distributions module to return a predictable cdf value. +// cdf returns 0.01 so that p-value = 2 * 0.01 = 0.02 +jest.mock('distributions', () => { + class MockStudentt { + cdf(_x: number): number { + return 0.01; + } + } + return { + __esModule: true, + default: { Studentt: MockStudentt }, + }; +}); + +const mockData: DataEntry[] = [ + { + group: ['group-A'], + values: [ + { x: 1, y: 10 }, + { x: 2, y: 20 }, + ], + }, + { + group: ['group-B'], + values: [ + { x: 1, y: 15 }, + { x: 2, y: 25 }, + ], + }, +]; + +const defaultProps = { + alpha: 0.05, + data: mockData, + groups: ['category'], + liftValPrec: 4, + metric: 'revenue', + pValPrec: 6, +}; + +test('renders the metric name as an h3 heading', async () => { + render(); + + await waitFor(() => { + expect(screen.getByText('revenue')).toBeInTheDocument(); + }); + + const heading = screen.getByText('revenue'); + expect(heading.tagName).toBe('H3'); +}); + +test('renders a table with the correct column headers', async () => { + render(); + + await waitFor(() => { + expect(screen.getByRole('table')).toBeInTheDocument(); + }); + + expect(screen.getByText('category')).toBeInTheDocument(); + expect(screen.getByText('p-value')).toBeInTheDocument(); + expect(screen.getByText('Lift %')).toBeInTheDocument(); + expect(screen.getByText('Significant')).toBeInTheDocument(); +}); + +test('renders group columns matching the groups prop', async () => { + const multiGroupData: DataEntry[] = [ + { + group: ['group-A', 'sub-1'], + values: [{ x: 1, y: 10 }], + }, + { + group: ['group-B', 'sub-2'], + values: [{ x: 1, y: 15 }], + }, + ]; + + render( + , + ); + + await waitFor(() => { + expect(screen.getByRole('table')).toBeInTheDocument(); + }); + + expect(screen.getByText('category')).toBeInTheDocument(); + expect(screen.getByText('subcategory')).toBeInTheDocument(); +}); + +test('first row is treated as control by default and shows "control" for p-value and lift columns', async () => { + render(); + + // After componentDidMount, the first row should be control + await waitFor(() => { + const controlTexts = screen.getAllByText('control'); + // The control row has "control" in pValue and liftValue columns + expect(controlTexts.length).toBeGreaterThanOrEqual(2); + }); +}); + +test('computes lift values correctly for non-control rows', async () => { + // Control (group-A): sum of y = 10 + 20 = 30 + // group-B: sum of y = 15 + 25 = 40 + // Lift = ((40 - 30) / 30) * 100 = 33.3333% + // With liftValPrec=4 => "33.3333" + render(); + + await waitFor(() => { + expect(screen.getByText('33.3333')).toBeInTheDocument(); + }); +}); + +test('computes p-value using the mocked distributions module', async () => { + // Mock cdf returns 0.01, so p-value = 2 * 0.01 = 0.02 + // With pValPrec=6 => "0.020000" + render(); + + await waitFor(() => { + expect(screen.getByText('0.020000')).toBeInTheDocument(); + }); +}); + +test('marks non-control row as significant when p-value is below alpha', async () => { + // p-value = 0.02 < alpha = 0.05, so significance is true + render(); + + await waitFor(() => { + expect(screen.getByText('true')).toBeInTheDocument(); + }); +}); + +test('marks non-control row as not significant when p-value is above alpha', async () => { + // p-value = 0.02 > alpha = 0.01, so significance is false + render(); + + await waitFor(() => { + expect(screen.getByText('false')).toBeInTheDocument(); + }); +}); + +test('returns NaN lift when control values sum to zero (division by zero guard)', async () => { + const zeroControlData: DataEntry[] = [ + { + group: ['zero-group'], + values: [ + { x: 1, y: 0 }, + { x: 2, y: 0 }, + ], + }, + { + group: ['other-group'], + values: [ + { x: 1, y: 10 }, + { x: 2, y: 20 }, + ], + }, + ]; + + render(); + + // The lift computation: ((sumValues - sumControl) / sumControl) * 100 + // = ((30 - 0) / 0) * 100 = Infinity + // Infinity.toFixed(4) in jsdom returns "NaN", and the component renders it. + // The getLiftStatus method classifies this as "invalid" (NaN or non-finite). + await waitFor(() => { + expect(screen.getByText('NaN')).toBeInTheDocument(); + }); +}); + +test('throws an error when groups array is empty', () => { + // Suppress React error boundary console output for this test + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => { + render(); + }).toThrow('Group by param is required'); + + consoleSpy.mockRestore(); +}); + +test('clicking a non-control row changes it to the new control', async () => { + render(); + + // Wait for initial render with group-A as control + await waitFor(() => { + expect(screen.getByText('group-A')).toBeInTheDocument(); + expect(screen.getByText('group-B')).toBeInTheDocument(); + }); + + // Initially group-A is control, so its row shows "control" in p-value and lift columns. + // The non-control row (group-B) shows computed values. + await waitFor(() => { + expect(screen.getByText('33.3333')).toBeInTheDocument(); + }); + + // Click the group-B row to make it the new control. + // The row containing "group-B" text is what we need to click. + const groupBCell = screen.getByText('group-B'); + const groupBRow = groupBCell.closest('tr'); + expect(groupBRow).not.toBeNull(); + fireEvent.click(groupBRow!); + + // After clicking, group-B becomes control. + // group-A lift: ((30 - 40) / 40) * 100 = -25.0000 + await waitFor(() => { + expect(screen.getByText('-25.0000')).toBeInTheDocument(); + }); +}); + +test('renders group name data in the table cells', async () => { + render(); + + await waitFor(() => { + expect(screen.getByText('group-A')).toBeInTheDocument(); + expect(screen.getByText('group-B')).toBeInTheDocument(); + }); +}); + +test('renders with three data rows and computes values for each non-control row', async () => { + const threeRowData: DataEntry[] = [ + { + group: ['control-group'], + values: [ + { x: 1, y: 10 }, + { x: 2, y: 10 }, + ], + }, + { + group: ['test-group-1'], + values: [ + { x: 1, y: 15 }, + { x: 2, y: 15 }, + ], + }, + { + group: ['test-group-2'], + values: [ + { x: 1, y: 20 }, + { x: 2, y: 20 }, + ], + }, + ]; + + render(); + + await waitFor(() => { + expect(screen.getByText('control-group')).toBeInTheDocument(); + expect(screen.getByText('test-group-1')).toBeInTheDocument(); + expect(screen.getByText('test-group-2')).toBeInTheDocument(); + }); + + // control-group: sum = 20 + // test-group-1: sum = 30, lift = ((30-20)/20)*100 = 50.0000 + // test-group-2: sum = 40, lift = ((40-20)/20)*100 = 100.0000 + await waitFor(() => { + expect(screen.getByText('50.0000')).toBeInTheDocument(); + expect(screen.getByText('100.0000')).toBeInTheDocument(); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx b/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx index 58f831fca4a..3402c1ec9a1 100644 --- a/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx +++ b/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import cloudLayout from 'd3-cloud'; import { scaleLinear } from 'd3-scale'; import { seed, CategoricalColorNamespace } from '@superset-ui/core'; @@ -81,18 +81,7 @@ export interface WordCloudProps extends WordCloudVisualProps { colorScheme: string; } -export interface WordCloudState { - words: Word[]; - scaleFactor: number; -} - -const defaultProps: Required = { - encoding: {}, - rotation: 'flat', -}; - -type FullWordCloudProps = WordCloudProps & - typeof defaultProps & { theme: SupersetTheme }; +type FullWordCloudProps = WordCloudProps & { theme: SupersetTheme }; const SCALE_FACTOR_STEP = 0.5; const MAX_SCALE_FACTOR = 3; @@ -196,61 +185,80 @@ class SimpleEncoder { } } -class WordCloud extends PureComponent { - static defaultProps = defaultProps; +function WordCloud({ + data, + encoding = {}, + width, + height, + rotation = 'flat', + sliceId, + colorScheme, + theme, +}: FullWordCloudProps) { + const [words, setWords] = useState([]); + const [scaleFactor] = useState(1); + const isMountedRef = useRef(true); - isComponentMounted = false; + // Store previous props for comparison + const prevPropsRef = useRef<{ + data: PlainObject[]; + encoding: Partial; + width: number; + height: number; + rotation: RotationType; + } | null>(null); - createEncoder = (encoding?: Partial): SimpleEncoder => - new SimpleEncoder(encoding ?? {}, { - color: this.props.theme.colorTextLabel, - fontFamily: this.props.theme.fontFamily, - fontSize: 20, - fontWeight: 'bold', - text: '', - }); + const createEncoder = useCallback( + (enc?: Partial): SimpleEncoder => + new SimpleEncoder(enc ?? {}, { + color: theme.colorTextLabel, + fontFamily: theme.fontFamily, + fontSize: 20, + fontWeight: 'bold', + text: '', + }), + [theme.colorTextLabel, theme.fontFamily], + ); - 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(); + const setWordsIfMounted = useCallback((newWords: Word[]) => { + if (isMountedRef.current) { + setWords(newWords); } - } + }, []); - componentWillUnmount() { - this.isComponentMounted = false; - } + const generateCloud = useCallback( + ( + encoder: SimpleEncoder, + currentScaleFactor: number, + isValid: (word: Word[]) => boolean, + ) => { + cloudLayout() + .size([width * currentScaleFactor, height * currentScaleFactor]) + .words(data.map((d: Word) => ({ ...d }))) + .padding(5) + .rotate(ROTATION[rotation] || ROTATION.flat) + .text((d: PlainObject) => encoder.getText(d)) + .font((d: PlainObject) => encoder.getFontFamily(d)) + .fontWeight((d: PlainObject) => encoder.getFontWeight(d)) + .fontSize((d: PlainObject) => encoder.getFontSize(d)) + .on('end', (cloudWords: Word[]) => { + if (isValid(cloudWords) || currentScaleFactor > MAX_SCALE_FACTOR) { + setWordsIfMounted(cloudWords); + } else { + generateCloud( + encoder, + currentScaleFactor + SCALE_FACTOR_STEP, + isValid, + ); + } + }) + .start(); + }, + [data, width, height, rotation, setWordsIfMounted], + ); - setWords(words: Word[]) { - if (this.isComponentMounted) { - this.setState({ words }); - } - } - - update() { - const { data, encoding } = this.props; - - const encoder = this.createEncoder(encoding); + const update = useCallback(() => { + const encoder = createEncoder(encoding); encoder.setDomainFromDataset(data); const sortedData = [...data].sort( @@ -262,73 +270,71 @@ class WordCloud extends PureComponent { ); const topResults = sortedData.slice(0, topResultsCount); - this.generateCloud(encoder, 1, (words: Word[]) => + generateCloud(encoder, 1, (cloudWords: Word[]) => topResults.every((d: PlainObject) => - words.find(({ text }) => encoder.getText(d) === text), + cloudWords.find(({ text }) => encoder.getText(d) === text), ), ); - } + }, [data, encoding, createEncoder, generateCloud]); - generateCloud( - encoder: SimpleEncoder, - scaleFactor: number, - isValid: (word: Word[]) => boolean, - ) { - const { data, width, height, rotation } = this.props; + // Component mount/unmount tracking + useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + }; + }, []); - cloudLayout() - .size([width * scaleFactor, height * scaleFactor]) - .words(data.map((d: Word) => ({ ...d }))) - .padding(5) - .rotate(ROTATION[rotation] || ROTATION.flat) - .text((d: PlainObject) => encoder.getText(d)) - .font((d: PlainObject) => encoder.getFontFamily(d)) - .fontWeight((d: PlainObject) => encoder.getFontWeight(d)) - .fontSize((d: PlainObject) => encoder.getFontSize(d)) - .on('end', (words: Word[]) => { - if (isValid(words) || scaleFactor > MAX_SCALE_FACTOR) { - this.setWords(words); - } else { - this.generateCloud(encoder, scaleFactor + SCALE_FACTOR_STEP, isValid); - } - }) - .start(); - } + // Initial update on mount and when dependencies change + useEffect(() => { + const prevProps = prevPropsRef.current; + const shouldUpdate = + !prevProps || + !isEqual(prevProps.data, data) || + !isEqual(prevProps.encoding, encoding) || + prevProps.width !== width || + prevProps.height !== height || + prevProps.rotation !== rotation; - render() { - const { scaleFactor, words } = this.state; - const { width, height, encoding, sliceId, colorScheme } = this.props; + if (shouldUpdate) { + update(); + } - const encoder = this.createEncoder(encoding); + prevPropsRef.current = { data, encoding, width, height, rotation }; + }, [data, encoding, width, height, rotation, update]); - const colorFn = CategoricalColorNamespace.getScale(colorScheme); - const viewBoxWidth = width * scaleFactor; - const viewBoxHeight = height * scaleFactor; + const encoder = useMemo( + () => createEncoder(encoding), + [createEncoder, encoding], + ); - return ( - - - {words.map(w => ( - - {w.text} - - ))} - - - ); - } + const colorFn = CategoricalColorNamespace.getScale(colorScheme); + const viewBoxWidth = width * scaleFactor; + const viewBoxHeight = height * scaleFactor; + + return ( + + + {words.map(w => ( + + {w.text} + + ))} + + + ); } export default withTheme(WordCloud);