From dc9a9ef5f4cf4d8edf5ba3061caa6e03e75ce59f Mon Sep 17 00:00:00 2001 From: "Dae Kun (DK) Kwon" Date: Mon, 24 Jul 2023 17:23:37 -0400 Subject: [PATCH 01/10] independent axis truncation for Histogram Viz --- .../HistogramVisualization.tsx | 40 ++++++++++++++----- .../lib/core/hooks/computeDefaultAxisRange.ts | 7 +++- .../src/lib/core/utils/default-axis-range.ts | 16 +++++--- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx index f6ab0d355c..74333dc9d6 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx @@ -562,6 +562,9 @@ function HistogramViz(props: VisualizationProps) { [data] ); + // Note: defaultIndependentRange in the Histogram Viz should keep its initial range + // regardless of the change of the data to ensure the truncation behavior + // Thus, pass an additional prop to useDefaultAxisRange() if Histogram Viz const defaultIndependentRange = useDefaultAxisRange( xAxisVariable, vizConfig.independentAxisValueSpec === 'Full' @@ -572,7 +575,9 @@ function HistogramViz(props: VisualizationProps) { ? undefined : independentAxisMinMax?.max, undefined, - vizConfig.independentAxisValueSpec + vizConfig.independentAxisValueSpec, + // pass true for histogramViz (default is false) + true ); // separate minPosMax from dependentMinPosMax @@ -731,16 +736,29 @@ function HistogramViz(props: VisualizationProps) { truncationConfigIndependentAxisMax, truncationConfigDependentAxisMin, truncationConfigDependentAxisMax, - } = truncationConfig( - { - ...defaultUIState, // using annotated range, NOT the actual data - ...(minPosMax != null && minPosMax.min != null && minPosMax.max != null - ? { dependentAxisRange: minPosMax } - : {}), - }, - vizConfig, - {}, // no overrides - true // use inclusive less than equal for the range min + } = useMemo( + () => + truncationConfig( + { + ...defaultUIState, // using annotated range, NOT the actual data + ...(minPosMax != null && + minPosMax.min != null && + minPosMax.max != null + ? { dependentAxisRange: minPosMax } + : {}), + }, + vizConfig, + {}, // no overrides + true // use inclusive less than equal for the range min + ), + [ + defaultUIState, + dependentMinPosMax, + vizConfig.independentAxisRange, + vizConfig.dependentAxisRange, + vizConfig.independentAxisValueSpec, + vizConfig.dependentAxisValueSpec, + ] ); // axis range control diff --git a/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts b/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts index 5c9c0c9c27..fa7a0cbb08 100755 --- a/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts +++ b/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts @@ -22,7 +22,9 @@ export function useDefaultAxisRange( max?: number | string, /** are we using a log scale */ logScale?: boolean, - axisRangeSpec = 'Full' + axisRangeSpec = 'Full', + // check histogramViz + histogramViz: boolean = false ): NumberOrDateRange | undefined { const defaultAxisRange = useMemo(() => { // Check here to make sure number ranges (min, minPos, max) came with number variables @@ -45,7 +47,8 @@ export function useDefaultAxisRange( minPos, max, logScale, - axisRangeSpec + axisRangeSpec, + histogramViz ); // 4 significant figures diff --git a/packages/libs/eda/src/lib/core/utils/default-axis-range.ts b/packages/libs/eda/src/lib/core/utils/default-axis-range.ts index bb1e61823d..1788d28a00 100755 --- a/packages/libs/eda/src/lib/core/utils/default-axis-range.ts +++ b/packages/libs/eda/src/lib/core/utils/default-axis-range.ts @@ -12,14 +12,16 @@ export function numberDateDefaultAxisRange( observedMax: number | string | undefined, /** are we using a log scale */ logScale?: boolean, - axisRangeSpec = 'Full' + axisRangeSpec = 'Full', + histogramViz: boolean = false ): NumberOrDateRange | undefined { if (Variable.is(variable)) { if (variable.type === 'number' || variable.type === 'integer') { const defaults = variable.distributionDefaults; if (logScale && observedMinPos == null) return undefined; // return nothing - there will be no plottable data anyway - // set default range of Custom to be Auto-zoom - return axisRangeSpec === 'Full' + // set default range of Custom to be Auto-zoom and check Histogram Viz + return axisRangeSpec === 'Full' || + (histogramViz && axisRangeSpec === 'Custom') ? { min: logScale && @@ -39,7 +41,7 @@ export function numberDateDefaultAxisRange( (min([ defaults.displayRangeMin ?? 0, defaults.rangeMin, - observedMin as number, + observedMin, ]) as number), max: max([ defaults.displayRangeMax, @@ -56,7 +58,8 @@ export function numberDateDefaultAxisRange( } else if (variable.type === 'date') { const defaults = variable.distributionDefaults; // considering axis range control option such as Full, Auto-zoom, and Custom for date type - return axisRangeSpec === 'Full' + return axisRangeSpec === 'Full' || + (histogramViz && axisRangeSpec === 'Custom') ? defaults.displayRangeMin != null && defaults.displayRangeMax != null ? { min: @@ -126,7 +129,8 @@ export function numberDateDefaultAxisRange( variable.displayRangeMin != null && variable.displayRangeMax != null ) { - return axisRangeSpec === 'Full' + return axisRangeSpec === 'Full' || + (histogramViz && axisRangeSpec === 'Custom') ? { min: logScale ? (observedMinPos as number) From de54a5d2f58661a50c8908b5b0150d1e03b84bd4 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Thu, 27 Jul 2023 17:48:43 -0400 Subject: [PATCH 02/10] Add legend to volcano plots --- .../VolcanoPlotVisualization.tsx | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx index b018d1fcd1..ec16db8428 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx @@ -35,6 +35,8 @@ import VolcanoSVG from './selectorIcons/VolcanoSVG'; import { NumberOrDate } from '@veupathdb/components/lib/types/general'; import { DifferentialAbundanceConfig } from '../../computations/plugins/differentialabundance'; import { yellow } from '@material-ui/core/colors'; +import PlotLegend from '@veupathdb/components/lib/components/plotControls/PlotLegend'; +import { significanceColors } from '@veupathdb/components/lib/types/plots'; // end imports @@ -189,8 +191,37 @@ function VolcanoPlotViz(props: VisualizationProps) { // TODO const controlsNode = <> ; - // TODO - const legendNode = {}; + const legendNode = data.value && ( + + ); // TODO const tableGroupNode = <> ; @@ -230,7 +261,7 @@ function VolcanoPlotViz(props: VisualizationProps) { /> */} Date: Mon, 31 Jul 2023 22:56:52 -0400 Subject: [PATCH 03/10] address feedbacks --- .../components/filter/HistogramFilter.tsx | 4 +- .../HistogramVisualization.tsx | 145 +++++++++++++++++- .../lib/core/hooks/computeDefaultAxisRange.ts | 7 +- .../lib/core/utils/axis-range-calculations.ts | 3 +- .../src/lib/core/utils/default-axis-range.ts | 12 +- 5 files changed, 148 insertions(+), 23 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx b/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx index fd195ec5ab..2136ee74b0 100755 --- a/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx @@ -129,6 +129,7 @@ export function HistogramFilter(props: Props) { }, [variableUISettings, uiStateKey, defaultUIState]); const uiStateForData = useDebounce(uiState, 1000); const subsettingClient = useSubsettingClient(); + const getData = useCallback( async ( dataParams: UIState @@ -181,6 +182,7 @@ export function HistogramFilter(props: Props) { variable.type ), ]; + const binWidth: NumberOrTimeDelta = NumberVariable.is(variable) ? dataParams.binWidth : { @@ -796,7 +798,7 @@ function HistogramPlotWithControls({ ); } -function distributionResponseToDataSeries( +export function distributionResponseToDataSeries( name: string, response: DistributionResponse, color: string, diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx index cb073bc373..0e02ceedf7 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx @@ -99,7 +99,10 @@ import { truncationConfig } from '../../../utils/truncation-config-utils'; // use Notification for truncation warning message import Notification from '@veupathdb/components/lib/components/widgets//Notification'; import AxisRangeControl from '@veupathdb/components/lib/components/plotControls/AxisRangeControl'; -import { UIState } from '../../filter/HistogramFilter'; +import { + UIState, + distributionResponseToDataSeries, +} from '../../filter/HistogramFilter'; // change defaultIndependentAxisRange to hook import { useDefaultAxisRange } from '../../../hooks/computeDefaultAxisRange'; import { @@ -124,6 +127,12 @@ import { ResetButtonCoreUI } from '../../ResetButton'; import { FloatingHistogramExtraProps } from '../../../../map/analysis/hooks/plugins/histogram'; import { useFindOutputEntity } from '../../../hooks/findOutputEntity'; +import { getDistribution } from '../../filter/util'; +import { DistributionResponse } from '../../../api/SubsettingClient'; +import { useSubsettingClient } from '../../../hooks/workspace'; +import { red } from '../../filter/colors'; +import { min, max } from 'lodash'; + export type HistogramDataWithCoverageStatistics = ( | HistogramData | FacetedData @@ -442,6 +451,85 @@ function HistogramViz(props: VisualizationProps) { [options, providedOverlayVariable, providedOverlayVariableDescriptor] ); + // get distribution data + const subsettingClient = useSubsettingClient(); + + const getData = useCallback(async () => { + if (vizConfig.xAxisVariable != null && xAxisVariable != null) { + const [displayRangeMin, displayRangeMax, binWidth, binUnits] = + NumberVariable.is(xAxisVariable) + ? [ + xAxisVariable.distributionDefaults.displayRangeMin ?? + xAxisVariable.distributionDefaults.rangeMin, + xAxisVariable.distributionDefaults.displayRangeMax ?? + xAxisVariable.distributionDefaults.rangeMax, + xAxisVariable.distributionDefaults.binWidth, + undefined, + ] + : [ + (xAxisVariable as DateVariable).distributionDefaults + .displayRangeMin ?? + (xAxisVariable as DateVariable).distributionDefaults.rangeMin, + (xAxisVariable as DateVariable).distributionDefaults + .displayRangeMax ?? + (xAxisVariable as DateVariable).distributionDefaults.rangeMax, + (xAxisVariable as DateVariable).distributionDefaults.binWidth, + (xAxisVariable as DateVariable).distributionDefaults.binUnits, + ]; + + const distribution = await getDistribution( + { + entityId: vizConfig.xAxisVariable?.entityId ?? '', + variableId: vizConfig.xAxisVariable?.variableId ?? '', + filters: filters, + }, + (filters) => { + return subsettingClient.getDistribution( + studyMetadata.id, + vizConfig.xAxisVariable?.entityId ?? '', + vizConfig.xAxisVariable?.variableId ?? '', + { + valueSpec: 'count', + filters, + binSpec: { + // Note: technically any arbitrary values can be used here for displayRangeMin/Max + // but used more accurate value anyway + displayRangeMin: DateVariable.is(xAxisVariable) + ? displayRangeMin + 'T00:00:00Z' + : displayRangeMin, + displayRangeMax: DateVariable.is(xAxisVariable) + ? displayRangeMax + 'T00:00:00Z' + : displayRangeMax, + binWidth: binWidth ?? 1, + binUnits: binUnits, + }, + } + ); + } + ); + + // return series using foreground response + const series = { + series: [ + distributionResponseToDataSeries( + 'Subset', + distribution.foreground, + red, + NumberVariable.is(xAxisVariable) ? 'number' : 'date' + ), + ], + }; + + return series; + } + + return undefined; + }, [filters, xAxisVariable, vizConfig.xAxisVariable]); + + const getDistributionData = usePromise( + useCallback(() => getData(), [getData]) + ); + const dataRequestConfig: DataRequestConfig = useDeepValue( pick(vizConfig, [ 'valueSpec', @@ -470,6 +558,10 @@ function HistogramViz(props: VisualizationProps) { ) return undefined; + // wait till getDistributionData is ready + if (getDistributionData.pending || getDistributionData.value == null) + return undefined; + if ( !variablesAreUnique([ xAxisVariable, @@ -565,13 +657,52 @@ function HistogramViz(props: VisualizationProps) { computation.descriptor.type, overlayEntity, facetEntity, + getDistributionData.pending, + getDistributionData.value, ]) ); - const independentAxisMinMax = useMemo( - () => histogramDefaultIndependentAxisMinMax(data), - [data] - ); + // Note: Histogram distribution data contains statistical values such as summary.min/max, + // however, it does not fully respect multiple filters. + // Similarly, distribution data also partially reflect filtered data. + // A solution is to compute both min/max values from data-based and summary-based ones, + // then take max of min values and min of max values, + // which will result in correct min/max value for multiple filters + // More specifically, data-based min and summary-based max are correct values + const dataBasedIndependentAxisMinMax = useMemo(() => { + return histogramDefaultIndependentAxisMinMax(getDistributionData); + }, [getDistributionData]); + + const summaryBasedIndependentAxisMinMax = useMemo(() => { + if (getDistributionData.value != null) + return { + min: DateVariable.is(xAxisVariable) + ? ( + (getDistributionData?.value?.series[0]?.summary?.min as string) ?? + '' + ).split('T')[0] + : getDistributionData?.value?.series[0]?.summary?.min, + max: DateVariable.is(xAxisVariable) + ? ( + (getDistributionData?.value?.series[0]?.summary?.max as string) ?? + '' + ).split('T')[0] + : getDistributionData?.value?.series[0]?.summary?.max, + }; + }, [getDistributionData]); + + const independentAxisMinMax = useMemo(() => { + return { + min: max([ + dataBasedIndependentAxisMinMax?.min, + summaryBasedIndependentAxisMinMax?.min, + ]), + max: min([ + dataBasedIndependentAxisMinMax?.max, + summaryBasedIndependentAxisMinMax?.max, + ]), + }; + }, [getDistributionData]); // Note: defaultIndependentRange in the Histogram Viz should keep its initial range // regardless of the change of the data to ensure the truncation behavior @@ -586,9 +717,7 @@ function HistogramViz(props: VisualizationProps) { ? undefined : independentAxisMinMax?.max, undefined, - vizConfig.independentAxisValueSpec, - // pass true for histogramViz (default is false) - true + vizConfig.independentAxisValueSpec ); // separate minPosMax from dependentMinPosMax diff --git a/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts b/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts index fa7a0cbb08..5c9c0c9c27 100755 --- a/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts +++ b/packages/libs/eda/src/lib/core/hooks/computeDefaultAxisRange.ts @@ -22,9 +22,7 @@ export function useDefaultAxisRange( max?: number | string, /** are we using a log scale */ logScale?: boolean, - axisRangeSpec = 'Full', - // check histogramViz - histogramViz: boolean = false + axisRangeSpec = 'Full' ): NumberOrDateRange | undefined { const defaultAxisRange = useMemo(() => { // Check here to make sure number ranges (min, minPos, max) came with number variables @@ -47,8 +45,7 @@ export function useDefaultAxisRange( minPos, max, logScale, - axisRangeSpec, - histogramViz + axisRangeSpec ); // 4 significant figures diff --git a/packages/libs/eda/src/lib/core/utils/axis-range-calculations.ts b/packages/libs/eda/src/lib/core/utils/axis-range-calculations.ts index 42c41036fa..b2789cd045 100755 --- a/packages/libs/eda/src/lib/core/utils/axis-range-calculations.ts +++ b/packages/libs/eda/src/lib/core/utils/axis-range-calculations.ts @@ -14,7 +14,8 @@ import { VariableMapping } from '../api/DataClient/types'; // calculate min/max of default independent axis range export function histogramDefaultIndependentAxisMinMax( - data: PromiseHookState + // use distribution data + data: PromiseHookState ) { if (isFaceted(data.value)) { const facetMinMaxes = diff --git a/packages/libs/eda/src/lib/core/utils/default-axis-range.ts b/packages/libs/eda/src/lib/core/utils/default-axis-range.ts index 1788d28a00..8c5e6a44f8 100755 --- a/packages/libs/eda/src/lib/core/utils/default-axis-range.ts +++ b/packages/libs/eda/src/lib/core/utils/default-axis-range.ts @@ -12,16 +12,14 @@ export function numberDateDefaultAxisRange( observedMax: number | string | undefined, /** are we using a log scale */ logScale?: boolean, - axisRangeSpec = 'Full', - histogramViz: boolean = false + axisRangeSpec = 'Full' ): NumberOrDateRange | undefined { if (Variable.is(variable)) { if (variable.type === 'number' || variable.type === 'integer') { const defaults = variable.distributionDefaults; if (logScale && observedMinPos == null) return undefined; // return nothing - there will be no plottable data anyway // set default range of Custom to be Auto-zoom and check Histogram Viz - return axisRangeSpec === 'Full' || - (histogramViz && axisRangeSpec === 'Custom') + return axisRangeSpec === 'Full' ? { min: logScale && @@ -58,8 +56,7 @@ export function numberDateDefaultAxisRange( } else if (variable.type === 'date') { const defaults = variable.distributionDefaults; // considering axis range control option such as Full, Auto-zoom, and Custom for date type - return axisRangeSpec === 'Full' || - (histogramViz && axisRangeSpec === 'Custom') + return axisRangeSpec === 'Full' ? defaults.displayRangeMin != null && defaults.displayRangeMax != null ? { min: @@ -129,8 +126,7 @@ export function numberDateDefaultAxisRange( variable.displayRangeMin != null && variable.displayRangeMax != null ) { - return axisRangeSpec === 'Full' || - (histogramViz && axisRangeSpec === 'Custom') + return axisRangeSpec === 'Full' ? { min: logScale ? (observedMinPos as number) From 3aee76a5fed853d240f2815bd0d95687250a9fb6 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Tue, 1 Aug 2023 12:44:58 -0400 Subject: [PATCH 04/10] Refactor to add counts to legend --- .../libs/components/src/plots/VolcanoPlot.tsx | 123 +++--------- .../libs/components/src/types/plots/addOns.ts | 15 +- .../components/src/types/plots/volcanoplot.ts | 2 + .../VolcanoPlotVisualization.tsx | 178 ++++++++++++++++-- 4 files changed, 207 insertions(+), 111 deletions(-) diff --git a/packages/libs/components/src/plots/VolcanoPlot.tsx b/packages/libs/components/src/plots/VolcanoPlot.tsx index 451d1f4f6b..a0b6f4edb1 100755 --- a/packages/libs/components/src/plots/VolcanoPlot.tsx +++ b/packages/libs/components/src/plots/VolcanoPlot.tsx @@ -5,12 +5,12 @@ import { useImperativeHandle, useRef, } from 'react'; -import { significanceColors } from '../types/plots'; import { VolcanoPlotData, VolcanoPlotDataPoint, } from '../types/plots/volcanoplot'; import { NumberRange } from '../types/general'; +import { SignificanceColors } from '../types/plots'; import { XYChart, Axis, @@ -22,7 +22,6 @@ import { AnnotationLabel, } from '@visx/xychart'; import { Group } from '@visx/group'; -import { max, min } from 'lodash'; import { gridStyles, thresholdLineStyles, @@ -38,6 +37,11 @@ import { ToImgopts } from 'plotly.js'; import { DEFAULT_CONTAINER_HEIGHT } from './PlotlyPlot'; import domToImage from 'dom-to-image'; +export interface RawDataMinMaxValues { + x: NumberRange; + y: NumberRange; +} + export interface VolcanoPlotProps { /** Data for the plot. An array of VolcanoPlotDataPoints */ data: VolcanoPlotData; @@ -70,6 +74,8 @@ export interface VolcanoPlotProps { containerStyles?: CSSProperties; /** shall we show the loading spinner? */ showSpinner?: boolean; + /** used to determine truncation logic */ + rawDataMinMaxValues?: RawDataMinMaxValues; } const EmptyVolcanoPlotData: VolcanoPlotData = []; @@ -122,6 +128,7 @@ function VolcanoPlot(props: VolcanoPlotProps, ref: Ref) { comparisonLabels, truncationBarFill, showSpinner = false, + rawDataMinMaxValues, } = props; // Use ref forwarding to enable screenshotting of the plot for thumbnail versions. @@ -138,87 +145,26 @@ function VolcanoPlot(props: VolcanoPlotProps, ref: Ref) { [] ); - /** - * Find mins and maxes of the data and for the plot. - * The standard x axis is the log2 fold change. The standard - * y axis is -log10 raw p value. - */ - - // Find maxes and mins of the data itself - const dataXMin = min(data.map((d) => Number(d.log2foldChange))) ?? 0; - const dataXMax = max(data.map((d) => Number(d.log2foldChange))) ?? 0; - const dataYMin = min(data.map((d) => Number(d.pValue))) ?? 0; - const dataYMax = max(data.map((d) => Number(d.pValue))) ?? 0; - - // Determine mins, maxes of axes in the plot. - // These are different than the data mins/maxes because - // of the log transform and the little bit of padding, or because axis ranges - // are supplied. - let xAxisMin: number; - let xAxisMax: number; - let yAxisMin: number; - let yAxisMax: number; - const AXIS_PADDING_FACTOR = 0.05; // The padding ensures we don't clip off part of the glyphs that represent - // the most extreme points. We could have also used d3.scale.nice but then we dont have precise control of where - // the extremes are, which is important for user-defined ranges and truncation bars. + // Set maxes and mins of the data itself from rawDataMinMaxValues prop + const dataXMin = rawDataMinMaxValues?.x.min ?? 0; + const dataXMax = rawDataMinMaxValues?.x.max ?? 0; + const dataYMin = rawDataMinMaxValues?.y.min ?? 0; + const dataYMax = rawDataMinMaxValues?.y.max ?? 0; - // X axis - if (independentAxisRange) { - xAxisMin = independentAxisRange.min; - xAxisMax = independentAxisRange.max; - } else { - if (dataXMin && dataXMax) { - // We can use the dataMin and dataMax here because we don't have a further transform - xAxisMin = dataXMin; - xAxisMax = dataXMax; - // Add a little padding to prevent clipping the glyph representing the extreme points - xAxisMin = xAxisMin - (xAxisMax - xAxisMin) * AXIS_PADDING_FACTOR; - xAxisMax = xAxisMax + (xAxisMax - xAxisMin) * AXIS_PADDING_FACTOR; - } else { - xAxisMin = 0; - xAxisMax = 0; - } - } - - // Y axis - if (dependentAxisRange) { - yAxisMin = dependentAxisRange.min; - yAxisMax = dependentAxisRange.max; - } else { - if (dataYMin && dataYMax) { - // Standard volcano plots have -log10(raw p value) as the y axis - yAxisMin = -Math.log10(dataYMax); - yAxisMax = -Math.log10(dataYMin); - // Add a little padding to prevent clipping the glyph representing the extreme points - yAxisMin = yAxisMin - (yAxisMax - yAxisMin) * AXIS_PADDING_FACTOR; - yAxisMax = yAxisMax + (yAxisMax - yAxisMin) * AXIS_PADDING_FACTOR; - } else { - yAxisMin = 0; - yAxisMax = 0; - } - } + // Set mins, maxes of axes in the plot using axis range props + const xAxisMin = independentAxisRange?.min ?? 0; + const xAxisMax = independentAxisRange?.max ?? 0; + const yAxisMin = dependentAxisRange?.min ?? 0; + const yAxisMax = dependentAxisRange?.max ?? 0; /** * Accessors - tell visx which value of the data point we should use and where. */ // For the actual volcano plot data - // Only return data if the points fall within the specified range! Otherwise they'll show up on the plot. const dataAccessors = { - xAccessor: (d: VolcanoPlotDataPoint) => { - const log2foldChange = Number(d?.log2foldChange); - - return log2foldChange <= xAxisMax && log2foldChange >= xAxisMin - ? log2foldChange - : null; - }, - yAccessor: (d: VolcanoPlotDataPoint) => { - const transformedPValue = -Math.log10(Number(d?.pValue)); - - return transformedPValue <= yAxisMax && transformedPValue >= yAxisMin - ? transformedPValue - : null; - }, + xAccessor: (d: VolcanoPlotDataPoint) => Number(d?.log2foldChange), + yAccessor: (d: VolcanoPlotDataPoint) => -Math.log10(Number(d?.pValue)), }; // For all other situations where we need to access point values. For example @@ -362,15 +308,7 @@ function VolcanoPlot(props: VolcanoPlotProps, ref: Ref) { dataKey={'data'} // unique key data={data} // data as an array of obejcts (points). Accessed with dataAccessors {...dataAccessors} - colorAccessor={(d) => { - return assignSignificanceColor( - Number(d.log2foldChange), - Number(d.pValue), - significanceThreshold, - log2FoldChangeThreshold, - significanceColors - ); - }} + colorAccessor={(d) => d.significanceColor} /> @@ -433,35 +371,30 @@ function VolcanoPlot(props: VolcanoPlotProps, ref: Ref) { /** * Assign color to point based on significance and magnitude change thresholds */ -function assignSignificanceColor( +export function assignSignificanceColor( log2foldChange: number, pValue: number, significanceThreshold: number, log2FoldChangeThreshold: number, - significanceColors: string[] // Assuming the order is [insignificant, high (up regulated), low (down regulated)] + significanceColors: SignificanceColors ) { - // Name indices of the significanceColors array for easier accessing. - const INSIGNIFICANT = 0; - const HIGH = 1; - const LOW = 2; - // Test 1. If the y value is higher than the significance threshold, just return not significant if (pValue >= significanceThreshold) { - return significanceColors[INSIGNIFICANT]; + return significanceColors['inconclusive']; } // Test 2. So the y is significant. Is the x larger than the positive foldChange threshold? if (log2foldChange >= log2FoldChangeThreshold) { - return significanceColors[HIGH]; + return significanceColors['high']; } // Test 3. Is the x value lower than the negative foldChange threshold? if (log2foldChange <= -log2FoldChangeThreshold) { - return significanceColors[LOW]; + return significanceColors['low']; } // If we're still here, it must be a non significant point. - return significanceColors[INSIGNIFICANT]; + return significanceColors['inconclusive']; } export default forwardRef(VolcanoPlot); diff --git a/packages/libs/components/src/types/plots/addOns.ts b/packages/libs/components/src/types/plots/addOns.ts index 4c40d47ac0..f57965365c 100644 --- a/packages/libs/components/src/types/plots/addOns.ts +++ b/packages/libs/components/src/types/plots/addOns.ts @@ -1,11 +1,10 @@ /** * Additional reusable modules to extend PlotProps and PlotData props */ - import { CSSProperties } from 'react'; import { BarLayoutOptions, OrientationOptions } from '.'; import { scaleLinear } from 'd3-scale'; -import { interpolateLab, extent, range } from 'd3'; +import { interpolateLab, range } from 'd3'; import { rgb, lab } from 'd3-color'; /** PlotProps addons */ @@ -283,8 +282,16 @@ export const gradientConvergingColorscaleMap = scaleLinear() .range(ConvergingGradientColorscale) .interpolate(interpolateLab); -// Significance colors (not significant, high, low) -export const significanceColors = ['#B5B8B4', '#AC3B4E', '#0E8FAB']; +export type SignificanceColors = { + inconclusive: string; + high: string; + low: string; +}; +export const significanceColors: SignificanceColors = { + inconclusive: '#B5B8B4', + high: '#AC3B4E', + low: '#0E8FAB', +}; /** truncated axis flags */ export type AxisTruncationAddon = { diff --git a/packages/libs/components/src/types/plots/volcanoplot.ts b/packages/libs/components/src/types/plots/volcanoplot.ts index e235abcb01..4e40169265 100755 --- a/packages/libs/components/src/types/plots/volcanoplot.ts +++ b/packages/libs/components/src/types/plots/volcanoplot.ts @@ -9,6 +9,8 @@ export type VolcanoPlotDataPoint = { adjustedPValue?: string; // Used for tooltip pointID?: string; + // Used to determine color of data point in the plot + significanceColor?: string; }; export type VolcanoPlotData = Array; diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx index ec16db8428..fe9caf4acb 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx @@ -1,10 +1,12 @@ // load scatter plot component import VolcanoPlot, { VolcanoPlotProps, + assignSignificanceColor, + RawDataMinMaxValues, } from '@veupathdb/components/lib/plots/VolcanoPlot'; import * as t from 'io-ts'; -import { useCallback } from 'react'; +import { useCallback, useState, useMemo } from 'react'; import { usePromise } from '../../../hooks/promise'; import { useUpdateThumbnailEffect } from '../../../hooks/thumbnails'; @@ -37,11 +39,18 @@ import { DifferentialAbundanceConfig } from '../../computations/plugins/differen import { yellow } from '@material-ui/core/colors'; import PlotLegend from '@veupathdb/components/lib/components/plotControls/PlotLegend'; import { significanceColors } from '@veupathdb/components/lib/types/plots'; - +import { NumberRange } from '../../../types/general'; +import { max, min } from 'lodash'; // end imports const DEFAULT_SIG_THRESHOLD = 0.05; const DEFAULT_FC_THRESHOLD = 2; +/** + * The padding ensures we don't clip off part of the glyphs that represent the most extreme points. + * We could have also used d3.scale.nice but then we dont have precise control of where the extremes + * are, which is important for user-defined ranges and truncation bars. + */ +const AXIS_PADDING_FACTOR = 0.05; const plotContainerStyles = { width: 750, @@ -150,6 +159,146 @@ function VolcanoPlotViz(props: VisualizationProps) { ]) ); + /** + * Find mins and maxes of the data and for the plot. + * The standard x axis is the log2 fold change. The standard + * y axis is -log10 raw p value. + */ + + // Find maxes and mins of the data itself + const rawDataMinMaxValues: RawDataMinMaxValues = useMemo(() => { + if (!data.value) + return { + x: { min: 0, max: 0 }, + y: { min: 0, max: 0 }, + }; + const dataXMin = min(data.value.map((d) => Number(d.log2foldChange))) ?? 0; + const dataXMax = max(data.value.map((d) => Number(d.log2foldChange))) ?? 0; + const dataYMin = min(data.value.map((d) => Number(d.pValue))) ?? 0; + const dataYMax = max(data.value.map((d) => Number(d.pValue))) ?? 0; + return { + x: { min: dataXMin, max: dataXMax }, + y: { min: dataYMin, max: dataYMax }, + }; + }, [data.value]); + + // Determine mins, maxes of axes in the plot. These are different than the data mins/maxes because + // of the log transform and the little bit of padding, or because axis ranges are supplied. + // NOTE: this state may be unnecessary depending on how we implement user-controlled axis ranges + const [xAxisRange, setXAxisRange] = + useState(undefined); + const independentAxisRange = useMemo(() => { + if (!data.value) return undefined; + if (xAxisRange) { + return xAxisRange; + } else { + const { + x: { min: dataXMin, max: dataXMax }, + } = rawDataMinMaxValues; + if (dataXMin && dataXMax) { + // We can use the dataMin and dataMax here because we don't have a further transform + // Add a little padding to prevent clipping the glyph representing the extreme points + return { + min: dataXMin - (dataXMax - dataXMin) * AXIS_PADDING_FACTOR, + max: dataXMax + (dataXMax - dataXMin) * AXIS_PADDING_FACTOR, + }; + } else { + return { min: 0, max: 0 }; + } + } + }, [data.value, xAxisRange, rawDataMinMaxValues]); + + // NOTE: this state may be unnecessary depending on how we implement user-controlled axis ranges + const [yAxisRange, setYAxisRange] = + useState(undefined); + const dependentAxisRange = useMemo(() => { + if (!data.value) return undefined; + if (yAxisRange) { + return yAxisRange; + } else { + const { + y: { min: dataYMin, max: dataYMax }, + } = rawDataMinMaxValues; + if (dataYMin && dataYMax) { + // Standard volcano plots have -log10(raw p value) as the y axis + const yAxisMin = -Math.log10(dataYMax); + const yAxisMax = -Math.log10(dataYMin); + // Add a little padding to prevent clipping the glyph representing the extreme points + return { + min: yAxisMin - (yAxisMax - yAxisMin) * AXIS_PADDING_FACTOR, + max: yAxisMax + (yAxisMax - yAxisMin) * AXIS_PADDING_FACTOR, + }; + } else { + return { min: 0, max: 0 }; + } + } + }, [data.value, yAxisRange, rawDataMinMaxValues]); + + const significanceThreshold = + vizConfig.significanceThreshold ?? DEFAULT_SIG_THRESHOLD; + const log2FoldChangeThreshold = + vizConfig.log2FoldChangeThreshold ?? DEFAULT_FC_THRESHOLD; + + /** + * Let's filter out data that falls outside of the plot axis ranges and then + * assign a significance color to the visible data + * This version of the data will get passed to the VolcanoPlot component + */ + const finalData = useMemo(() => { + if (data.value && independentAxisRange && dependentAxisRange) { + // Only return data if the points fall within the specified range! Otherwise they'll show up on the plot. + return data.value + .filter((d) => { + const log2foldChange = Number(d?.log2foldChange); + const transformedPValue = -Math.log10(Number(d?.pValue)); + return ( + log2foldChange <= independentAxisRange.max && + log2foldChange >= independentAxisRange.min && + transformedPValue <= dependentAxisRange.max && + transformedPValue >= dependentAxisRange.min + ); + }) + .map((d) => ({ + ...d, + significanceColor: assignSignificanceColor( + Number(d.log2foldChange), + Number(d.pValue), + significanceThreshold, + log2FoldChangeThreshold, + significanceColors + ), + })); + } + }, [ + data.value, + independentAxisRange, + dependentAxisRange, + significanceThreshold, + log2FoldChangeThreshold, + significanceColors, + ]); + + // TODO: Given that I cannot figure out how to type this correctly, I suspect there's a simpler, cleaner + // way to get the counts. + const countsData = useMemo(() => { + if (!finalData) return; + return finalData.reduce( + (prev: any, curr: any) => { + if (curr.significanceColor) { + return { + ...prev, + [curr.significanceColor]: ++prev[curr.significanceColor], + }; + } + }, + { + [significanceColors['inconclusive']]: 0, + [significanceColors['high']]: 0, + [significanceColors['low']]: 0, + } + ); + }, [finalData, significanceColors]); + const plotRef = useUpdateThumbnailEffect( updateThumbnail, plotContainerStyles, @@ -175,14 +324,17 @@ function VolcanoPlotViz(props: VisualizationProps) { : []; const volcanoPlotProps: VolcanoPlotProps = { - data: data.value ? Object.values(data.value) : [], + data: finalData ? Object.values(finalData) : [], markerBodyOpacity: vizConfig.markerBodyOpacity ?? 0.5, - significanceThreshold: vizConfig.significanceThreshold ?? 0.05, - log2FoldChangeThreshold: vizConfig.log2FoldChangeThreshold ?? 3, + significanceThreshold, + log2FoldChangeThreshold, containerStyles: plotContainerStyles, comparisonLabels: comparisonLabels, showSpinner: data.pending, truncationBarFill: yellow[300], + independentAxisRange, + dependentAxisRange, + rawDataMinMaxValues, }; // @ts-ignore @@ -191,32 +343,34 @@ function VolcanoPlotViz(props: VisualizationProps) { // TODO const controlsNode = <> ; - const legendNode = data.value && ( + const legendNode = finalData && countsData && ( Date: Thu, 3 Aug 2023 11:19:36 -0400 Subject: [PATCH 05/10] Clean up hook dependency arrays to please ESLint --- .../implementations/VolcanoPlotVisualization.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx index fe9caf4acb..11fbe4c149 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx @@ -147,9 +147,6 @@ function VolcanoPlotViz(props: VisualizationProps) { computeJobStatus, filteredCounts.pending, filteredCounts.value, - entities, - dataElementConstraints, - dataElementDependencyOrder, filters, studyId, computationConfiguration, @@ -275,7 +272,6 @@ function VolcanoPlotViz(props: VisualizationProps) { dependentAxisRange, significanceThreshold, log2FoldChangeThreshold, - significanceColors, ]); // TODO: Given that I cannot figure out how to type this correctly, I suspect there's a simpler, cleaner @@ -297,7 +293,7 @@ function VolcanoPlotViz(props: VisualizationProps) { [significanceColors['low']]: 0, } ); - }, [finalData, significanceColors]); + }, [finalData]); const plotRef = useUpdateThumbnailEffect( updateThumbnail, From 890b70cfea2090209c3479ec3d47bb72b503fbe8 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Thu, 3 Aug 2023 11:20:43 -0400 Subject: [PATCH 06/10] Require rawDataMinMaxValues and update stories accordingly --- .../libs/components/src/plots/VolcanoPlot.tsx | 8 ++-- .../src/stories/plots/VolcanoPlot.stories.tsx | 45 +++++++++++++++---- .../stories/plots/VolcanoPlotRef.stories.tsx | 38 ++++++++++++++-- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/packages/libs/components/src/plots/VolcanoPlot.tsx b/packages/libs/components/src/plots/VolcanoPlot.tsx index a0b6f4edb1..a5374c42e4 100755 --- a/packages/libs/components/src/plots/VolcanoPlot.tsx +++ b/packages/libs/components/src/plots/VolcanoPlot.tsx @@ -75,7 +75,7 @@ export interface VolcanoPlotProps { /** shall we show the loading spinner? */ showSpinner?: boolean; /** used to determine truncation logic */ - rawDataMinMaxValues?: RawDataMinMaxValues; + rawDataMinMaxValues: RawDataMinMaxValues; } const EmptyVolcanoPlotData: VolcanoPlotData = []; @@ -146,10 +146,8 @@ function VolcanoPlot(props: VolcanoPlotProps, ref: Ref) { ); // Set maxes and mins of the data itself from rawDataMinMaxValues prop - const dataXMin = rawDataMinMaxValues?.x.min ?? 0; - const dataXMax = rawDataMinMaxValues?.x.max ?? 0; - const dataYMin = rawDataMinMaxValues?.y.min ?? 0; - const dataYMax = rawDataMinMaxValues?.y.max ?? 0; + const { min: dataXMin, max: dataXMax } = rawDataMinMaxValues.x; + const { min: dataYMin, max: dataYMax } = rawDataMinMaxValues.y; // Set mins, maxes of axes in the plot using axis range props const xAxisMin = independentAxisRange?.min ?? 0; diff --git a/packages/libs/components/src/stories/plots/VolcanoPlot.stories.tsx b/packages/libs/components/src/stories/plots/VolcanoPlot.stories.tsx index c7597de82c..96070111b0 100755 --- a/packages/libs/components/src/stories/plots/VolcanoPlot.stories.tsx +++ b/packages/libs/components/src/stories/plots/VolcanoPlot.stories.tsx @@ -5,6 +5,8 @@ import { getNormallyDistributedRandomNumber } from './ScatterPlot.storyData'; import { VolcanoPlotData } from '../../types/plots/volcanoplot'; import { NumberRange } from '../../types/general'; import { yellow } from '@veupathdb/coreui/lib/definitions/colors'; +import { assignSignificanceColor } from '../../plots/VolcanoPlot'; +import { significanceColors } from '../../types/plots'; export default { title: 'Plots/VolcanoPlot', @@ -99,14 +101,40 @@ const Template: Story = (args) => { // Process input data. Take the object of arrays and turn it into // an array of data points. Note the backend will do this for us! const volcanoDataPoints: VolcanoPlotData = - args.data.volcanoplot.log2foldChange.map((l2fc, index) => { - return { - log2foldChange: l2fc, - pValue: args.data.volcanoplot.pValue[index], - adjustedPValue: args.data.volcanoplot.adjustedPValue[index], - pointID: args.data.volcanoplot.pointID[index], - }; - }); + args.data.volcanoplot.log2foldChange + .map((l2fc, index) => { + return { + log2foldChange: l2fc, + pValue: args.data.volcanoplot.pValue[index], + adjustedPValue: args.data.volcanoplot.adjustedPValue[index], + pointID: args.data.volcanoplot.pointID[index], + }; + }) + .map((d) => ({ + ...d, + significanceColor: assignSignificanceColor( + Number(d.log2foldChange), + Number(d.pValue), + args.significanceThreshold, + args.log2FoldChangeThreshold, + significanceColors + ), + })); + + const rawDataMinMaxValues = { + x: { + min: + Math.min(...volcanoDataPoints.map((d) => Number(d.log2foldChange))) ?? + 0, + max: + Math.max(...volcanoDataPoints.map((d) => Number(d.log2foldChange))) ?? + 0, + }, + y: { + min: Math.min(...volcanoDataPoints.map((d) => Number(d.pValue))) ?? 0, + max: Math.max(...volcanoDataPoints.map((d) => Number(d.pValue))) ?? 0, + }, + }; const volcanoPlotProps: VolcanoPlotProps = { data: volcanoDataPoints, @@ -118,6 +146,7 @@ const Template: Story = (args) => { dependentAxisRange: args.dependentAxisRange, truncationBarFill: args.truncationBarFill, showSpinner: args.showSpinner, + rawDataMinMaxValues, }; return ( diff --git a/packages/libs/components/src/stories/plots/VolcanoPlotRef.stories.tsx b/packages/libs/components/src/stories/plots/VolcanoPlotRef.stories.tsx index 8070890015..aacca4e493 100644 --- a/packages/libs/components/src/stories/plots/VolcanoPlotRef.stories.tsx +++ b/packages/libs/components/src/stories/plots/VolcanoPlotRef.stories.tsx @@ -1,11 +1,13 @@ import { useEffect } from 'react'; import { useState } from 'react'; import { useRef } from 'react'; -import { Story, Meta } from '@storybook/react/types-6-0'; +import { Story } from '@storybook/react/types-6-0'; import VolcanoPlot, { VolcanoPlotProps } from '../../plots/VolcanoPlot'; import { range } from 'lodash'; import { VolcanoPlotData } from '../../types/plots/volcanoplot'; import { getNormallyDistributedRandomNumber } from './ScatterPlot.storyData'; +import { assignSignificanceColor } from '../../plots/VolcanoPlot'; +import { significanceColors } from '../../types/plots'; export default { title: 'Plots/VolcanoPlot', @@ -63,15 +65,40 @@ const Template: Story = (args) => { }, []); // Wrangle data to get it into the nice form for plot component. - const volcanoDataPoints: VolcanoPlotData = - data.volcanoplot.log2foldChange.map((l2fc, index) => { + const volcanoDataPoints: VolcanoPlotData = data.volcanoplot.log2foldChange + .map((l2fc, index) => { return { log2foldChange: l2fc, pValue: data.volcanoplot.pValue[index], adjustedPValue: data.volcanoplot.adjustedPValue[index], pointID: data.volcanoplot.pointID[index], }; - }); + }) + .map((d) => ({ + ...d, + significanceColor: assignSignificanceColor( + Number(d.log2foldChange), + Number(d.pValue), + args.significanceThreshold, + args.log2FoldChangeThreshold, + significanceColors + ), + })); + + const rawDataMinMaxValues = { + x: { + min: + Math.min(...volcanoDataPoints.map((d) => Number(d.log2foldChange))) ?? + 0, + max: + Math.max(...volcanoDataPoints.map((d) => Number(d.log2foldChange))) ?? + 0, + }, + y: { + min: Math.min(...volcanoDataPoints.map((d) => Number(d.pValue))) ?? 0, + max: Math.max(...volcanoDataPoints.map((d) => Number(d.pValue))) ?? 0, + }, + }; const volcanoPlotProps: VolcanoPlotProps = { data: volcanoDataPoints, @@ -79,6 +106,9 @@ const Template: Story = (args) => { log2FoldChangeThreshold: args.log2FoldChangeThreshold, markerBodyOpacity: args.markerBodyOpacity, comparisonLabels: args.comparisonLabels, + rawDataMinMaxValues, + independentAxisRange: { min: -9, max: 9 }, + dependentAxisRange: { min: 0, max: 9 }, }; return ( From fe71b000755ab49098c7c74830f381f82c0621ed Mon Sep 17 00:00:00 2001 From: Dave Falke Date: Thu, 3 Aug 2023 13:11:59 -0400 Subject: [PATCH 07/10] Filter out unknown attributes from user pref and step details. (#402) --- .../src/Utils/UserPreferencesUtils.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/libs/wdk-client/src/Utils/UserPreferencesUtils.ts b/packages/libs/wdk-client/src/Utils/UserPreferencesUtils.ts index bbfdd1efff..996425c9c3 100644 --- a/packages/libs/wdk-client/src/Utils/UserPreferencesUtils.ts +++ b/packages/libs/wdk-client/src/Utils/UserPreferencesUtils.ts @@ -17,6 +17,7 @@ import { AttributeSortingSpec, SearchConfig, } from '../Utils/WdkModel'; +import { partition } from 'lodash'; /* * TODO: this file should be updated to offer request/update/fulfill actions and handlers from request/update to fulfill. application store modules will call them, and reduce the fulfills into their store state @@ -61,7 +62,26 @@ export async function getResultTableColumnsPref( : columnsPref ? columnsPref.trim().split(/,\s*/) : question.defaultAttributes; - return uniq(fixedColumns.concat(columns)); + + const [knownColumns, unknownColumns] = partition( + columns, + (columnName) => columnName in recordClass.attributesMap + ); + + if (unknownColumns.length > 0) { + await wdkService.submitError( + new Error( + 'The following unknown attributes were encountered from either user preferences or step details.\n\n' + + 'Search name: ' + + searchName + + '\n' + + (step ? 'Step ID: ' + step.id + '\n' : '') + + 'Unknown attributes: ' + + unknownColumns.join(', ') + ) + ); + } + return uniq(fixedColumns.concat(knownColumns)); } export async function setResultTableColumnsPref( From b0917043c064adc73aa2a05dacb85b77bd30eeb1 Mon Sep 17 00:00:00 2001 From: Jeremy Myers Date: Thu, 3 Aug 2023 15:36:21 -0400 Subject: [PATCH 08/10] Simplify countsData logic --- .../VolcanoPlotVisualization.tsx | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx index 11fbe4c149..c393589e5a 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/VolcanoPlotVisualization.tsx @@ -274,25 +274,18 @@ function VolcanoPlotViz(props: VisualizationProps) { log2FoldChangeThreshold, ]); - // TODO: Given that I cannot figure out how to type this correctly, I suspect there's a simpler, cleaner - // way to get the counts. + // For the legend, we need the counts of each assigned significance value const countsData = useMemo(() => { if (!finalData) return; - return finalData.reduce( - (prev: any, curr: any) => { - if (curr.significanceColor) { - return { - ...prev, - [curr.significanceColor]: ++prev[curr.significanceColor], - }; - } - }, - { - [significanceColors['inconclusive']]: 0, - [significanceColors['high']]: 0, - [significanceColors['low']]: 0, - } - ); + const counts = { + [significanceColors['inconclusive']]: 0, + [significanceColors['high']]: 0, + [significanceColors['low']]: 0, + }; + for (const entry of finalData) { + counts[entry.significanceColor]++; + } + return counts; }, [finalData]); const plotRef = useUpdateThumbnailEffect( From a2c1cede5942561bfadcdb3591162043af848bde Mon Sep 17 00:00:00 2001 From: "Dae Kun (DK) Kwon" Date: Mon, 7 Aug 2023 13:52:59 -0400 Subject: [PATCH 09/10] address feedbacks --- .../lib/core/api/SubsettingClient/types.ts | 2 +- .../HistogramVisualization.tsx | 93 +++++++++---------- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/packages/libs/eda/src/lib/core/api/SubsettingClient/types.ts b/packages/libs/eda/src/lib/core/api/SubsettingClient/types.ts index c7ce822e3d..9b45c730e5 100644 --- a/packages/libs/eda/src/lib/core/api/SubsettingClient/types.ts +++ b/packages/libs/eda/src/lib/core/api/SubsettingClient/types.ts @@ -21,7 +21,7 @@ export const StudyResponse = type({ }); export interface DistributionRequestParams { - filters: Filter[]; + filters?: Filter[]; binSpec?: { displayRangeMin: number | string; displayRangeMax: number | string; diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx index 0e02ceedf7..079adf7c18 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/HistogramVisualization.tsx @@ -454,7 +454,7 @@ function HistogramViz(props: VisualizationProps) { // get distribution data const subsettingClient = useSubsettingClient(); - const getData = useCallback(async () => { + const getDistributionData = useCallback(async () => { if (vizConfig.xAxisVariable != null && xAxisVariable != null) { const [displayRangeMin, displayRangeMax, binWidth, binUnits] = NumberVariable.is(xAxisVariable) @@ -477,34 +477,26 @@ function HistogramViz(props: VisualizationProps) { (xAxisVariable as DateVariable).distributionDefaults.binUnits, ]; - const distribution = await getDistribution( + // try to call once + const distribution = await subsettingClient.getDistribution( + studyMetadata.id, + vizConfig.xAxisVariable?.entityId ?? '', + vizConfig.xAxisVariable?.variableId ?? '', { - entityId: vizConfig.xAxisVariable?.entityId ?? '', - variableId: vizConfig.xAxisVariable?.variableId ?? '', - filters: filters, - }, - (filters) => { - return subsettingClient.getDistribution( - studyMetadata.id, - vizConfig.xAxisVariable?.entityId ?? '', - vizConfig.xAxisVariable?.variableId ?? '', - { - valueSpec: 'count', - filters, - binSpec: { - // Note: technically any arbitrary values can be used here for displayRangeMin/Max - // but used more accurate value anyway - displayRangeMin: DateVariable.is(xAxisVariable) - ? displayRangeMin + 'T00:00:00Z' - : displayRangeMin, - displayRangeMax: DateVariable.is(xAxisVariable) - ? displayRangeMax + 'T00:00:00Z' - : displayRangeMax, - binWidth: binWidth ?? 1, - binUnits: binUnits, - }, - } - ); + valueSpec: 'count', + filters, + binSpec: { + // Note: technically any arbitrary values can be used here for displayRangeMin/Max + // but used more accurate value anyway + displayRangeMin: DateVariable.is(xAxisVariable) + ? displayRangeMin + 'T00:00:00Z' + : displayRangeMin, + displayRangeMax: DateVariable.is(xAxisVariable) + ? displayRangeMax + 'T00:00:00Z' + : displayRangeMax, + binWidth: binWidth ?? 1, + binUnits: binUnits, + }, } ); @@ -513,7 +505,7 @@ function HistogramViz(props: VisualizationProps) { series: [ distributionResponseToDataSeries( 'Subset', - distribution.foreground, + distribution, red, NumberVariable.is(xAxisVariable) ? 'number' : 'date' ), @@ -524,10 +516,11 @@ function HistogramViz(props: VisualizationProps) { } return undefined; - }, [filters, xAxisVariable, vizConfig.xAxisVariable]); + }, [filters, xAxisVariable, vizConfig.xAxisVariable, subsettingClient]); - const getDistributionData = usePromise( - useCallback(() => getData(), [getData]) + // need useCallback to avoid infinite loop + const distributionDataPromise = usePromise( + useCallback(() => getDistributionData(), [getDistributionData]) ); const dataRequestConfig: DataRequestConfig = useDeepValue( @@ -558,8 +551,11 @@ function HistogramViz(props: VisualizationProps) { ) return undefined; - // wait till getDistributionData is ready - if (getDistributionData.pending || getDistributionData.value == null) + // wait till distributionDataPromise is ready + if ( + distributionDataPromise.pending || + distributionDataPromise.value == null + ) return undefined; if ( @@ -657,8 +653,8 @@ function HistogramViz(props: VisualizationProps) { computation.descriptor.type, overlayEntity, facetEntity, - getDistributionData.pending, - getDistributionData.value, + distributionDataPromise.pending, + distributionDataPromise.value, ]) ); @@ -670,26 +666,29 @@ function HistogramViz(props: VisualizationProps) { // which will result in correct min/max value for multiple filters // More specifically, data-based min and summary-based max are correct values const dataBasedIndependentAxisMinMax = useMemo(() => { - return histogramDefaultIndependentAxisMinMax(getDistributionData); - }, [getDistributionData]); + return histogramDefaultIndependentAxisMinMax(distributionDataPromise); + }, [distributionDataPromise]); const summaryBasedIndependentAxisMinMax = useMemo(() => { - if (getDistributionData.value != null) + if ( + !distributionDataPromise.pending && + distributionDataPromise.value != null + ) return { min: DateVariable.is(xAxisVariable) ? ( - (getDistributionData?.value?.series[0]?.summary?.min as string) ?? - '' + (distributionDataPromise?.value?.series[0]?.summary + ?.min as string) ?? '' ).split('T')[0] - : getDistributionData?.value?.series[0]?.summary?.min, + : distributionDataPromise?.value?.series[0]?.summary?.min, max: DateVariable.is(xAxisVariable) ? ( - (getDistributionData?.value?.series[0]?.summary?.max as string) ?? - '' + (distributionDataPromise?.value?.series[0]?.summary + ?.max as string) ?? '' ).split('T')[0] - : getDistributionData?.value?.series[0]?.summary?.max, + : distributionDataPromise?.value?.series[0]?.summary?.max, }; - }, [getDistributionData]); + }, [distributionDataPromise]); const independentAxisMinMax = useMemo(() => { return { @@ -702,7 +701,7 @@ function HistogramViz(props: VisualizationProps) { summaryBasedIndependentAxisMinMax?.max, ]), }; - }, [getDistributionData]); + }, [distributionDataPromise]); // Note: defaultIndependentRange in the Histogram Viz should keep its initial range // regardless of the change of the data to ensure the truncation behavior From 551eb20d27b941f8facca6f02a97d02356621b68 Mon Sep 17 00:00:00 2001 From: Dave Falke Date: Tue, 8 Aug 2023 10:14:35 -0400 Subject: [PATCH 10/10] Add non-success response logging to `FetchClient` (#406) * Add callback option for non-success responses * Remove unused module * Add static method to set onNonSuccessResponse handler --- packages/libs/http-utils/src/FetchClient.ts | 38 +++- packages/libs/web-common/src/bootstrap.js | 6 + packages/libs/web-common/src/util/api.ts | 196 -------------------- 3 files changed, 42 insertions(+), 198 deletions(-) delete mode 100644 packages/libs/web-common/src/util/api.ts diff --git a/packages/libs/http-utils/src/FetchClient.ts b/packages/libs/http-utils/src/FetchClient.ts index 779a41a77e..c305aebef0 100644 --- a/packages/libs/http-utils/src/FetchClient.ts +++ b/packages/libs/http-utils/src/FetchClient.ts @@ -37,12 +37,25 @@ export interface FetchApiOptions { init?: RequestInit; /** Implementation of `fetch` function. Defaults to `window.fetch`. */ fetchApi?: Window['fetch']; + /** + * Callback that can be used for reporting errors. A Promise rejection will + * still occur. + */ + onNonSuccessResponse?: (error: Error) => void; +} + +class FetchClientError extends Error { + name = 'FetchClientError'; } export abstract class FetchClient { + /** Default callback used, if none is specified to constructor. */ + private static onNonSuccessResponse: FetchApiOptions['onNonSuccessResponse']; + protected readonly baseUrl: string; protected readonly init: RequestInit; protected readonly fetchApi: Window['fetch']; + protected readonly onNonSuccessResponse: FetchApiOptions['onNonSuccessResponse']; // Subclasses can set this to false to disable including a traceparent header with all requests. protected readonly includeTraceidHeader: boolean = true; @@ -50,6 +63,23 @@ export abstract class FetchClient { this.baseUrl = options.baseUrl; this.init = options.init ?? {}; this.fetchApi = options.fetchApi ?? window.fetch; + this.onNonSuccessResponse = + options.onNonSuccessResponse ?? FetchClient.onNonSuccessResponse; + } + + /** + * Set a default callback for all instances. Should only be called once. + */ + public static setOnNonSuccessResponse( + callback: FetchApiOptions['onNonSuccessResponse'] + ) { + if (this.onNonSuccessResponse) { + console.warn( + 'FetchClient.setOnNonSuccessResponse() should only be called once.' + ); + return; + } + this.onNonSuccessResponse = callback; } protected async fetch(apiRequest: ApiRequest): Promise { @@ -74,9 +104,13 @@ export abstract class FetchClient { return await transformResponse(responseBody); } - throw new Error( - `${response.status} ${response.statusText}${'\n'}${await response.text()}` + const fetchError = new FetchClientError( + `${request.method.toUpperCase()} ${request.url}: ${response.status} ${ + response.statusText + }${'\n'}${await response.text()}` ); + this.onNonSuccessResponse?.(fetchError); + throw fetchError; } } diff --git a/packages/libs/web-common/src/bootstrap.js b/packages/libs/web-common/src/bootstrap.js index 1a038707d6..5ebdeb64ab 100644 --- a/packages/libs/web-common/src/bootstrap.js +++ b/packages/libs/web-common/src/bootstrap.js @@ -16,6 +16,7 @@ import { debounce, identity, uniq, flow } from 'lodash'; // TODO Remove auth_tkt from url before proceeding +import { FetchClient } from '@veupathdb/http-utils'; import { initialize as initializeWdk_ } from '@veupathdb/wdk-client/lib/Core/main'; import * as WdkComponents from '@veupathdb/wdk-client/lib/Components'; import * as WdkControllers from '@veupathdb/wdk-client/lib/Controllers'; @@ -94,6 +95,11 @@ export function initialize(options = {}) { context.store.dispatch(loadSiteConfig(siteConfig)); + // Add non-success response handler for FetchClient instances + FetchClient.setOnNonSuccessResponse((error) => { + context.wdkService.submitError(error); + }); + return context; } diff --git a/packages/libs/web-common/src/util/api.ts b/packages/libs/web-common/src/util/api.ts deleted file mode 100644 index ea054617d2..0000000000 --- a/packages/libs/web-common/src/util/api.ts +++ /dev/null @@ -1,196 +0,0 @@ -import { mapValues, compose } from 'lodash/fp'; -import { - Decoder, - standardErrorReport, -} from '@veupathdb/wdk-client/lib/Utils/Json'; - -/* - * An "Api" is an abstraction for interacting with resources. - * - * There are two primary interfaces: `ApiRequest` and `ApiRequestHandler`. - * - * An `ApiRequest` represents a HTTP-like request for a resource. - * - * An `ApiRequestHandler` represents an implentation that can handle a request. - * Typically this will be based on the `fetch` API. - */ - -/** - * Represents an HTTP-like request for a resource. - */ -export interface ApiRequest { - /** Path to resource, relative to a fixed base url. */ - path: string; - /** Request method for resource. */ - method: string; - /** Body of request */ - body?: any; - /** Headers to add to the request. */ - headers?: Record; - /** Transform response body. This is a good place to do validation. */ - transformResponse: (body: unknown) => Promise; -} - -export interface ApiRequestCreator { - (...args: U): ApiRequest; -} - -export interface ApiRequestsObject { - [Key: string]: ApiRequestCreator; -} - -type ApiRequestToBound> = - R extends ApiRequestCreator - ? (...args: U) => Promise - : never; - -export type BoundApiRequestsObject = { - [P in keyof T]: T[P] extends ApiRequestCreator - ? (...args: B) => Promise - : never; -}; - -export function bindApiRequestCreators( - requestCreators: T, - handler: ApiRequestHandler -): BoundApiRequestsObject { - return mapValues( - (requestCreator) => compose(handler, requestCreator), - requestCreators - ) as BoundApiRequestsObject; -} - -// XXX Not sure if these belong here, since they are specific to an ApiRequestHandler - -/** Helper to create a request with a JSON body. */ -export function createJsonRequest(init: ApiRequest): ApiRequest { - return { - ...init, - body: JSON.stringify(init.body), - headers: { - ...init.headers, - 'Content-Type': 'application/json', - }, - }; -} - -/** Helper to create a request with a plain text body. */ -export function createPlainTextRequest(init: ApiRequest): ApiRequest { - return { - ...init, - headers: { - ...init.headers, - 'Content-Type': 'text/plain', - }, - }; -} - -/** Standard transformer that uses a `Json.ts` `decoder` type. */ -export function standardTransformer(decoder: Decoder) { - return async function transform(body: unknown): Promise { - const result = decoder(body); - if (result.status === 'ok') return result.value; - const report = `Expected ${result.expected}${ - result.context ? 'at _' + result.context : '' - }, but got ${JSON.stringify(result.value)}.`; - throw new Error('Could not decode response.\n' + report); - }; -} - -/** - * A function that takes an `ApiRequest` and returns a `Promise`. - */ -export interface ApiRequestHandler { - (request: ApiRequest): Promise; -} - -/** - * Options for a `fetch`-based request handler. - */ -export interface FetchApiOptions { - /** Base url for service endpoint. */ - baseUrl: string; - /** Global optoins for all requests. */ - init?: RequestInit; - /** Implementation of `fetch` function. Defaults to `window.fetch`. */ - fetchApi?: Window['fetch']; -} - -/** - * A `fetch`-based implentation of an `ApiRequestHandler`. - */ -export function createFetchApiRequestHandler( - options: FetchApiOptions -): ApiRequestHandler { - const { baseUrl, init = {}, fetchApi = window.fetch } = options; - return async function fetchApiRequestHandler( - apiRequest: ApiRequest - ): Promise { - const { transformResponse, path, body, ...restReq } = apiRequest; - const request = new Request(baseUrl + path, { - ...init, - ...restReq, - body: body, - headers: { - ...restReq.headers, - ...init.headers, - }, - }); - const response = await fetchApi(request); - // TODO Make this behavior configurable - if (response.ok) { - const responseBody = await fetchResponseBody(response); - - return await transformResponse(responseBody); - } - throw new Error( - `${response.status} ${response.statusText}${'\n'}${await response.text()}` - ); - }; -} - -export abstract class FetchClient { - protected readonly baseUrl: string; - protected readonly init: RequestInit; - protected readonly fetchApi: Window['fetch']; - - constructor(options: FetchApiOptions) { - this.baseUrl = options.baseUrl; - this.init = options.init ?? {}; - this.fetchApi = options.fetchApi ?? window.fetch; - } - - protected async fetch(apiRequest: ApiRequest): Promise { - const { baseUrl, init, fetchApi } = this; - const { transformResponse, path, body, ...restReq } = apiRequest; - const request = new Request(baseUrl + path, { - ...init, - ...restReq, - body: body, - headers: { - ...restReq.headers, - ...init.headers, - }, - }); - const response = await fetchApi(request); - // TODO Make this behavior configurable - if (response.ok) { - const responseBody = await fetchResponseBody(response); - - return await transformResponse(responseBody); - } - throw new Error( - `${response.status} ${response.statusText}${'\n'}${await response.text()}` - ); - } -} - -async function fetchResponseBody(response: Response) { - const contentType = response.headers.get('Content-Type'); - - return contentType == null - ? undefined - : contentType.startsWith('application/json') - ? response.json() - : response.text(); -}