From 1619749df41b37e5220d85ab3047692cac769f1b Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 11 Oct 2024 14:10:13 +0100 Subject: [PATCH 01/26] WIP but not working --- .../libs/eda/src/lib/core/api/queryClient.ts | 17 ++++++++++ .../components/filter/HistogramFilter.tsx | 7 ++-- .../eda/src/lib/core/hooks/cachedPromise.ts | 26 +++++++++++++++ .../libs/eda/src/lib/map/MapVeuContainer.tsx | 20 ++---------- .../src/lib/workspace/WorkspaceContainer.tsx | 32 +++++++++++-------- 5 files changed, 68 insertions(+), 34 deletions(-) create mode 100644 packages/libs/eda/src/lib/core/api/queryClient.ts create mode 100644 packages/libs/eda/src/lib/core/hooks/cachedPromise.ts diff --git a/packages/libs/eda/src/lib/core/api/queryClient.ts b/packages/libs/eda/src/lib/core/api/queryClient.ts new file mode 100644 index 0000000000..be92676418 --- /dev/null +++ b/packages/libs/eda/src/lib/core/api/queryClient.ts @@ -0,0 +1,17 @@ +import { QueryClient } from '@tanstack/react-query'; + +export const queryClient = new QueryClient({ + defaultOptions: { + queries: { + // This is similar behavior to our custom usePromise hook. + // It can be overridden on an individual basis, if needed. + keepPreviousData: true, + // We presume data will not go stale during the lifecycle of an application. + staleTime: Infinity, + // Do not attempt to retry if an error is encountered + retry: false, + // Do not referch when the browser tab is focused again + refetchOnWindowFocus: false, + }, + }, +}); 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 cd8e27c09d..ef38069b15 100755 --- a/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx @@ -24,7 +24,7 @@ import { getOrElse } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/function'; import { number, partial, TypeOf, boolean, type, intersection } from 'io-ts'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { usePromise } from '../../hooks/promise'; +import { useCachedPromise } from '../../hooks/cachedPromise'; import { AnalysisState } from '../../hooks/analysis'; import { useSubsettingClient } from '../../hooks/workspace'; import { DateRangeFilter, NumberRangeFilter } from '../../types/filter'; @@ -229,8 +229,9 @@ export function HistogramFilter(props: Props) { variable, ] ); - const data = usePromise( - useCallback(() => getData(uiStateForData), [getData, uiStateForData]) + const data = useCachedPromise( + () => getData(uiStateForData), + [getData, uiStateForData] ); const filter = filters?.find( diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts new file mode 100644 index 0000000000..93cf9895c6 --- /dev/null +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -0,0 +1,26 @@ +import { useMemo } from 'react'; +import { v4 as uuidv4 } from 'uuid'; +import { useQuery } from 'react-query'; +import { PromiseHookState } from './promise'; + +export function useCachedPromise( + task: () => Promise, + deps: any[] +): PromiseHookState { + // generate a serialisable key for react-query from a mix of data and class/function dependencies + const uniqueKey = useMemo(() => uuidv4(), deps); + // Using useQuery from react-query with the unique key + const { data, error, isLoading } = useQuery({ + queryKey: ['useCachedPromise', uniqueKey], + queryFn: task, + }); + + // Mapping the state from useQuery to PromiseHookState + const state: PromiseHookState = { + value: data, + pending: isLoading, + error: error, + }; + + return state; +} diff --git a/packages/libs/eda/src/lib/map/MapVeuContainer.tsx b/packages/libs/eda/src/lib/map/MapVeuContainer.tsx index 1b028f39ca..c504114aeb 100644 --- a/packages/libs/eda/src/lib/map/MapVeuContainer.tsx +++ b/packages/libs/eda/src/lib/map/MapVeuContainer.tsx @@ -6,7 +6,9 @@ import { useHistory, } from 'react-router'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { QueryClientProvider } from '@tanstack/react-query'; +import { queryClient } from '../core/api/queryClient'; + import { ReactQueryDevtools } from '@tanstack/react-query-devtools'; import { EDAAnalysisListContainer, EDAWorkspaceContainer } from '../core'; @@ -59,22 +61,6 @@ export function MapVeuContainer(mapVeuContainerProps: Props) { history.push(loginUrl); } - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - // This is similar behavior to our custom usePromise hook. - // It can be overridden on an individual basis, if needed. - keepPreviousData: true, - // We presume data will not go stale during the lifecycle of an application. - staleTime: Infinity, - // Do not attempt to retry if an error is encountered - retry: false, - // Do not referch when the browser tab is focused again - refetchOnWindowFocus: false, - }, - }, - }); - // This will get the matched path of the active parent route. // This is useful so we don't have to hardcode the path root. const { path } = useRouteMatch(); diff --git a/packages/libs/eda/src/lib/workspace/WorkspaceContainer.tsx b/packages/libs/eda/src/lib/workspace/WorkspaceContainer.tsx index 95ee3e9f58..39aa29afcf 100644 --- a/packages/libs/eda/src/lib/workspace/WorkspaceContainer.tsx +++ b/packages/libs/eda/src/lib/workspace/WorkspaceContainer.tsx @@ -15,6 +15,8 @@ import { VariableDescriptor } from '../core/types/variable'; import { cx, findFirstVariable } from './Utils'; import { makeStyles } from '@material-ui/core/styles'; +import { QueryClientProvider } from '@tanstack/react-query'; +import { queryClient } from '../core/api/queryClient'; const useStyles = makeStyles({ workspace: { @@ -73,19 +75,21 @@ export function WorkspaceContainer({ const classes = useStyles(); return ( - - {children} - + + + {children} + + ); } From 6a7f6d0c15b45c94facf7b676de723e6671c1a94 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 11 Oct 2024 19:17:09 +0100 Subject: [PATCH 02/26] react-query working now for subsetting --- .../core/components/filter/HistogramFilter.tsx | 16 +++++----------- .../lib/core/components/filter/MultiFilter.tsx | 12 ++++++------ .../lib/core/components/filter/TableFilter.tsx | 9 +++++---- .../libs/eda/src/lib/core/hooks/cachedPromise.ts | 10 +++------- 4 files changed, 19 insertions(+), 28 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 ef38069b15..e66eb9ffbf 100755 --- a/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/HistogramFilter.tsx @@ -126,13 +126,11 @@ export function HistogramFilter(props: Props) { getOrElse((): UIState => defaultUIState) ); }, [variableUISettings, uiStateKey, defaultUIState]); - const uiStateForData = useDebounce(uiState, 1000); + const dataParams = useDebounce(uiState, 1000); const subsettingClient = useSubsettingClient(); - const getData = useCallback( - async ( - dataParams: UIState - ): Promise< + const data = useCachedPromise( + async (): Promise< HistogramData & { variableId: string; entityId: string; @@ -220,18 +218,14 @@ export function HistogramFilter(props: Props) { }; }, [ + dataParams, otherFilters, entity.displayName, entity.displayNamePlural, entity.id, studyMetadata.id, - subsettingClient, variable, - ] - ); - const data = useCachedPromise( - () => getData(uiStateForData), - [getData, uiStateForData] + ] // used to have `subsettingClient` ); const filter = filters?.find( diff --git a/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx b/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx index aec12df37e..64e2192ce3 100644 --- a/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx @@ -9,7 +9,6 @@ import { } from '@veupathdb/wdk-client/lib/Components/AttributeFilter/Types'; import { AnalysisState } from '../../hooks/analysis'; -import { usePromise } from '../../hooks/promise'; import { useSubsettingClient } from '../../hooks/workspace'; import { MultiFilter as MultiFilterType } from '../../types/filter'; import { @@ -33,6 +32,7 @@ import { gray, red } from './colors'; import { debounce } from 'lodash'; import { isTableVariable } from './guards'; import { useDeepValue } from '../../hooks/immutability'; +import { useCachedPromise } from '../../hooks/cachedPromise'; export interface Props { analysisState: AnalysisState; @@ -193,8 +193,8 @@ export function MultiFilter(props: Props) { }, [_thisFilter, thisFilter]); // Counts retrieved from the backend, used for the table display. - const leafSummariesPromise = usePromise( - useCallback(() => { + const leafSummariesPromise = useCachedPromise( + () => { return Promise.all( leaves.map((leaf) => { const thisFilterWithoutLeaf = thisFilter && { @@ -258,15 +258,15 @@ export function MultiFilter(props: Props) { }); }) ); - }, [ + }, + [ thisFilter, otherFilters, leaves, entity.id, - subsettingClient, studyMetadata.id, variablesById, - ]) + ] // used to have `subsettingClient` ); // Sorted counts. This is done separately from retrieving the data so that diff --git a/packages/libs/eda/src/lib/core/components/filter/TableFilter.tsx b/packages/libs/eda/src/lib/core/components/filter/TableFilter.tsx index 6049f4b5e6..83467bcc66 100644 --- a/packages/libs/eda/src/lib/core/components/filter/TableFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/TableFilter.tsx @@ -5,7 +5,6 @@ import { getOrElse, map } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/function'; import { boolean, keyof, number, partial, string, type, TypeOf } from 'io-ts'; import { useCallback, useMemo } from 'react'; -import { usePromise } from '../../hooks/promise'; import { AnalysisState } from '../../hooks/analysis'; import { useSubsettingClient } from '../../hooks/workspace'; import { Filter } from '../../types/filter'; @@ -18,6 +17,7 @@ import { gray, red } from './colors'; // import axis label unit util import { variableDisplayWithUnit } from '../../utils/variable-display'; import { useDeepValue } from '../../hooks/immutability'; +import { useCachedPromise } from '../../hooks/cachedPromise'; type Props = { studyMetadata: StudyMetadata; @@ -80,8 +80,8 @@ export function TableFilter({ (f) => f.entityId !== entity.id || f.variableId !== variable.id ) ); - const tableSummary = usePromise( - useCallback(async () => { + const tableSummary = useCachedPromise( + async () => { const distribution = await getDistribution( { entityId: entity.id, @@ -127,7 +127,8 @@ export function TableFilter({ filteredEntitiesCount: distribution.foreground.statistics.numDistinctEntityRecords, }; - }, [entity.id, variable, otherFilters, subsettingClient, studyMetadata.id]) + }, + [entity.id, variable, otherFilters, studyMetadata.id] // used to have `subsettingClient` ); const activeField = useMemo( () => ({ diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 93cf9895c6..4ad673c67c 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -1,17 +1,13 @@ -import { useMemo } from 'react'; -import { v4 as uuidv4 } from 'uuid'; -import { useQuery } from 'react-query'; +import { useQuery } from '@tanstack/react-query'; import { PromiseHookState } from './promise'; export function useCachedPromise( task: () => Promise, - deps: any[] + queryKey: any[] | undefined ): PromiseHookState { - // generate a serialisable key for react-query from a mix of data and class/function dependencies - const uniqueKey = useMemo(() => uuidv4(), deps); // Using useQuery from react-query with the unique key const { data, error, isLoading } = useQuery({ - queryKey: ['useCachedPromise', uniqueKey], + queryKey: ['useCachedPromise', ...(queryKey ?? [])], queryFn: task, }); From 95aaead0c4d7bb49c553408953895223eac8742f Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 15:18:33 +0100 Subject: [PATCH 03/26] basic approach working but still WIP --- .../HistogramVisualization.tsx | 354 +++++++++--------- .../eda/src/lib/core/hooks/cachedPromise.ts | 11 +- 2 files changed, 180 insertions(+), 185 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 6c041e5b8d..5825281511 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 @@ -35,7 +35,6 @@ import { HistogramResponse, } from '../../../api/DataClient'; import DataClient from '../../../api/DataClient'; -import { usePromise } from '../../../hooks/promise'; import { useDataClient, useStudyMetadata, @@ -127,6 +126,7 @@ import { useOutputEntity } from '../../../hooks/findOutputEntity'; import { useSubsettingClient } from '../../../hooks/workspace'; import { red } from '../../filter/colors'; import { min, max } from 'lodash'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; export type HistogramDataWithCoverageStatistics = ( | HistogramData @@ -461,71 +461,76 @@ function HistogramViz(props: VisualizationProps) { // get distribution data const subsettingClient = useSubsettingClient(); - const distributionDataPromise = usePromise( - 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, - ]; - - // try to call once - const distribution = await 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, - red, - NumberVariable.is(xAxisVariable) ? 'number' : 'date' - ), - ], - }; + const distributionDataArgs: + | Parameters + | undefined = useMemo(() => { + if (vizConfig.xAxisVariable == null || xAxisVariable == null) return; + + 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, + ]; + + return [ + 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, + }, + }, + ]; + }, [vizConfig, xAxisVariable, studyMetadata]); + + const distributionDataPromise = useCachedPromise(async () => { + if (!distributionDataArgs) + throw new Error('distributionDataArgs is undefined'); + const distribution = await subsettingClient.getDistribution( + ...distributionDataArgs + ); - return series; - } + // return series using foreground response + const series = { + series: [ + distributionResponseToDataSeries( + 'Subset', + distribution, + red, + NumberVariable.is(xAxisVariable) ? 'number' : 'date' + ), + ], + }; - return undefined; - }, [filters, xAxisVariable, vizConfig.xAxisVariable, subsettingClient]) - ); + return series; + }, [distributionDataArgs, xAxisVariable]); // Note: Histogram distribution data contains statistical values such as summary.min/max, // however, it does not fully respect multiple filters. @@ -571,7 +576,7 @@ function HistogramViz(props: VisualizationProps) { summaryBasedIndependentAxisMinMax?.max, ]), }; - }, [distributionDataPromise]); + }, [dataBasedIndependentAxisMinMax, summaryBasedIndependentAxisMinMax]); // Note: defaultIndependentRange in the Histogram Viz should keep its initial range // regardless of the change of the data to ensure the truncation behavior @@ -589,138 +594,119 @@ function HistogramViz(props: VisualizationProps) { vizConfig.independentAxisValueSpec ); - const dataRequestConfig: DataRequestConfig = useDeepValue( - pick(vizConfig, [ - 'valueSpec', - 'independentAxisValueSpec', - 'binWidth', - 'binWidthTimeUnit', - 'valueSpec', - 'overlayVariable', - 'facetVariable', - 'xAxisVariable', - 'independentAxisRange', - 'showMissingness', - ]) - ); - - const data = usePromise( - useCallback(async (): Promise< - HistogramDataWithCoverageStatistics | undefined - > => { - if ( - dataRequestConfig.xAxisVariable == null || - xAxisVariable == null || - filteredCounts.pending || - filteredCounts.value == null - ) - return undefined; - - if ( - !variablesAreUnique([ + const dataRequestProps = + vizConfig.xAxisVariable == null || + xAxisVariable == null || + filteredCounts.pending || + filteredCounts.value == null + ? undefined // no data request will be made if undefined + : { + // pick only the props that should affect data requests + // e.g. changes in dependentAxisLogScale should NOT trigger new data + dataRequestConfig: pick(vizConfig, [ + 'valueSpec', + 'independentAxisValueSpec', + 'binWidth', + 'binWidthTimeUnit', + 'valueSpec', + 'overlayVariable', + 'facetVariable', + 'xAxisVariable', + 'independentAxisRange', + 'showMissingness', + ]), + filteredCounts: filteredCounts.value, xAxisVariable, - overlayVariable && (providedOverlayVariable ?? overlayVariable), - facetVariable, - ]) - ) - throw new Error(nonUniqueWarning); - - assertValidInputVariables( - inputs, - selectedVariables, - entities, - dataElementConstraints, - dataElementDependencyOrder - ); + }; - // We test this after assertValidInputVariables because - // that gives a useful message to users. Returning undefined doesn't. - if (outputEntity == null) return undefined; + const data = useCachedPromise(async (): Promise< + HistogramDataWithCoverageStatistics | undefined + > => { + if (!dataRequestProps) throw new Error('dataRequestProps is not defined'); - const params = getRequestParams( - studyId, - filters ?? [], - valueType, - dataRequestConfig, - xAxisVariable, - outputEntity, - defaultIndependentRange, - options?.getRequestParams - ); - const response = await dataClient.getHistogram( - computation.descriptor.type, - visualization.descriptor.type, - params - ); + const { dataRequestConfig, filteredCounts, xAxisVariable } = + dataRequestProps; - const showMissingOverlay = - dataRequestConfig.showMissingness && - hasIncompleteCases( - overlayEntity, - overlayVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - const showMissingFacet = - dataRequestConfig.showMissingness && - hasIncompleteCases( - facetEntity, - facetVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - - const overlayVocabulary = - (overlayVariable && options?.getOverlayVocabulary?.()) ?? - fixLabelsForNumberVariables( - overlayVariable?.vocabulary, - overlayVariable - ); - const facetVocabulary = fixLabelsForNumberVariables( - facetVariable?.vocabulary, - facetVariable - ); + if ( + !variablesAreUnique([ + xAxisVariable, + overlayVariable && (providedOverlayVariable ?? overlayVariable), + facetVariable, + ]) + ) + throw new Error(nonUniqueWarning); - return grayOutLastSeries( - substituteUnselectedToken( - reorderData( - histogramResponseToData( - response, - xAxisVariable, - overlayVariable, - facetVariable - ), - vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), - vocabularyWithMissingData(facetVocabulary, showMissingFacet) - ) - ), - showMissingOverlay - ); - }, [ - dataRequestConfig, - xAxisVariable, - outputEntity, - filteredCounts.pending, - filteredCounts.value, - overlayVariable, - facetVariable, + assertValidInputVariables( inputs, selectedVariables, entities, dataElementConstraints, - dataElementDependencyOrder, - filters, + dataElementDependencyOrder + ); + + // We test this after assertValidInputVariables because + // that gives a useful message to users. Returning undefined doesn't. TO DO FIX COMMENTS<<<<<< + if (outputEntity == null) throw new Error('this is crazy'); + + const params = getRequestParams( studyId, + filters ?? [], valueType, - dataClient, + dataRequestConfig, + xAxisVariable, + outputEntity, + defaultIndependentRange, + options?.getRequestParams + ); + const response = await dataClient.getHistogram( computation.descriptor.type, - overlayEntity, - facetEntity, visualization.descriptor.type, - ]) - ); + params + ); + + const showMissingOverlay = + dataRequestConfig.showMissingness && + hasIncompleteCases( + overlayEntity, + overlayVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + const showMissingFacet = + dataRequestConfig.showMissingness && + hasIncompleteCases( + facetEntity, + facetVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + + const overlayVocabulary = + (overlayVariable && options?.getOverlayVocabulary?.()) ?? + fixLabelsForNumberVariables(overlayVariable?.vocabulary, overlayVariable); + const facetVocabulary = fixLabelsForNumberVariables( + facetVariable?.vocabulary, + facetVariable + ); + + return grayOutLastSeries( + substituteUnselectedToken( + reorderData( + histogramResponseToData( + response, + xAxisVariable, + overlayVariable, + facetVariable + ), + vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), + vocabularyWithMissingData(facetVocabulary, showMissingFacet) + ) + ), + showMissingOverlay + ); + }, [dataRequestProps]); const [checkData, isEmptyData] = useMemo(() => { // controls need the bin info from just one facet (not an empty one) diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 4ad673c67c..46e6f972b4 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -1,14 +1,23 @@ import { useQuery } from '@tanstack/react-query'; import { PromiseHookState } from './promise'; +// This is a wrapper around `useQuery` to replace our `usePromise` hook. +// This will provide full client-side cacheing (cache size > 1!) +// It is not quite a drop-in replacement as the caching uses serialisable keys. +// + +// Note: the task may not return undefined. +// The task will not be executed while all items in the queryKey array are nullish +// (empty queryKey array not allowed) export function useCachedPromise( task: () => Promise, - queryKey: any[] | undefined + queryKey: [any, ...any[]] ): PromiseHookState { // Using useQuery from react-query with the unique key const { data, error, isLoading } = useQuery({ queryKey: ['useCachedPromise', ...(queryKey ?? [])], queryFn: task, + enabled: queryKey.every((val) => val != null), }); // Mapping the state from useQuery to PromiseHookState From 226d77566ef1ffc07422664349bddda1ae1a8ed6 Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 15:37:13 +0100 Subject: [PATCH 04/26] missing dep --- .../visualizations/implementations/HistogramVisualization.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5825281511..59c0742c1a 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 @@ -508,7 +508,7 @@ function HistogramViz(props: VisualizationProps) { }, }, ]; - }, [vizConfig, xAxisVariable, studyMetadata]); + }, [vizConfig, xAxisVariable, studyMetadata, filters]); const distributionDataPromise = useCachedPromise(async () => { if (!distributionDataArgs) From b3c0f5974ef3245b507b55d59590490b5df48a7a Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 15:38:20 +0100 Subject: [PATCH 05/26] another missing dep --- .../visualizations/implementations/HistogramVisualization.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 59c0742c1a..3d4f845db2 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 @@ -563,7 +563,7 @@ function HistogramViz(props: VisualizationProps) { } } return undefined; - }, [distributionDataPromise]); + }, [distributionDataPromise, xAxisVariable]); const independentAxisMinMax = useMemo(() => { return { From db8aa66fbae81b8c4b2d320ac0159d865fdaa38d Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 16:41:38 +0100 Subject: [PATCH 06/26] improve comments --- .../implementations/HistogramVisualization.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 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 3d4f845db2..08fcc91a24 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 @@ -644,9 +644,8 @@ function HistogramViz(props: VisualizationProps) { dataElementDependencyOrder ); - // We test this after assertValidInputVariables because - // that gives a useful message to users. Returning undefined doesn't. TO DO FIX COMMENTS<<<<<< - if (outputEntity == null) throw new Error('this is crazy'); + // sanity check for typescript - should never throw + if (outputEntity == null) throw new Error('outputEntity is not defined'); const params = getRequestParams( studyId, From d5a2c6dbd11cf4ffe2ca3410f8f5ff63a176dfa7 Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 16:55:46 +0100 Subject: [PATCH 07/26] solve userdb PATCH update issue with referential stability --- .../libs/eda/src/lib/core/hooks/cachedPromise.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 46e6f972b4..65a8fc15ae 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -1,5 +1,6 @@ import { useQuery } from '@tanstack/react-query'; import { PromiseHookState } from './promise'; +import { useMemo } from 'react'; // This is a wrapper around `useQuery` to replace our `usePromise` hook. // This will provide full client-side cacheing (cache size > 1!) @@ -21,11 +22,15 @@ export function useCachedPromise( }); // Mapping the state from useQuery to PromiseHookState - const state: PromiseHookState = { - value: data, - pending: isLoading, - error: error, - }; + // and return something stable + const state: PromiseHookState = useMemo( + () => ({ + value: data, + pending: isLoading, + error: error, + }), + [data, isLoading, error] + ); return state; } From deca1d98b72a9e1b52cc29f9cca894d0a4af46bc Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 17:55:05 +0100 Subject: [PATCH 08/26] spinner behaviour hopefully fixed --- packages/libs/eda/src/lib/core/hooks/cachedPromise.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 65a8fc15ae..e3f04297c0 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -14,11 +14,12 @@ export function useCachedPromise( task: () => Promise, queryKey: [any, ...any[]] ): PromiseHookState { - // Using useQuery from react-query with the unique key - const { data, error, isLoading } = useQuery({ + const enabled = queryKey.every((val) => val != null); + + const { data, error, isLoading, isFetching } = useQuery({ queryKey: ['useCachedPromise', ...(queryKey ?? [])], queryFn: task, - enabled: queryKey.every((val) => val != null), + enabled, }); // Mapping the state from useQuery to PromiseHookState @@ -26,10 +27,10 @@ export function useCachedPromise( const state: PromiseHookState = useMemo( () => ({ value: data, - pending: isLoading, + pending: enabled && (isLoading || isFetching), error: error, }), - [data, isLoading, error] + [data, enabled, isLoading, isFetching, error] ); return state; From cd3e13ef474cc15f5fa94aca76a450d0aaa445f9 Mon Sep 17 00:00:00 2001 From: Bob Date: Thu, 17 Oct 2024 23:00:40 +0100 Subject: [PATCH 09/26] cancel variable works and old data is flushed when appropriate --- .../libs/eda/src/lib/core/hooks/cachedPromise.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index e3f04297c0..916c13f5d9 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -3,13 +3,17 @@ import { PromiseHookState } from './promise'; import { useMemo } from 'react'; // This is a wrapper around `useQuery` to replace our `usePromise` hook. -// This will provide full client-side cacheing (cache size > 1!) +// +// It provides full client-side cacheing (cache size > 1!) // It is not quite a drop-in replacement as the caching uses serialisable keys. +// But on the plus side, it's not necessary to memoize the task function. // - -// Note: the task may not return undefined. // The task will not be executed while all items in the queryKey array are nullish // (empty queryKey array not allowed) +// +// Note: the task may not return undefined, but the hook will return `value: undefined` +// when disabled. +// export function useCachedPromise( task: () => Promise, queryKey: [any, ...any[]] @@ -20,17 +24,18 @@ export function useCachedPromise( queryKey: ['useCachedPromise', ...(queryKey ?? [])], queryFn: task, enabled, + keepPreviousData: enabled, }); // Mapping the state from useQuery to PromiseHookState // and return something stable const state: PromiseHookState = useMemo( () => ({ - value: data, + value: enabled ? data : undefined, pending: enabled && (isLoading || isFetching), error: error, }), - [data, enabled, isLoading, isFetching, error] + [data, enabled, isLoading || isFetching, error] ); return state; From 1a3ede4cb06dab7d915163838170fc9c9d67fa60 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 12:10:16 +0100 Subject: [PATCH 10/26] add cacheTime argument to useCachedPromise for the future --- packages/libs/eda/src/lib/core/hooks/cachedPromise.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 916c13f5d9..6d387d7e65 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -14,9 +14,12 @@ import { useMemo } from 'react'; // Note: the task may not return undefined, but the hook will return `value: undefined` // when disabled. // +// cacheTime is optional and in milliseconds - default from react-query is 5 minutes +// export function useCachedPromise( task: () => Promise, - queryKey: [any, ...any[]] + queryKey: [any, ...any[]], + cacheTime?: number ): PromiseHookState { const enabled = queryKey.every((val) => val != null); @@ -25,6 +28,7 @@ export function useCachedPromise( queryFn: task, enabled, keepPreviousData: enabled, + ...(cacheTime != null ? { cacheTime } : {}), }); // Mapping the state from useQuery to PromiseHookState From 4484b0c8b3f51f9112ca6aaa95365a5ee58ab19d Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 14:40:02 +0100 Subject: [PATCH 11/26] map supporting plots now behave correctly --- .../implementations/HistogramVisualization.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 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 08fcc91a24..cf09dbc7ef 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 @@ -617,6 +617,8 @@ function HistogramViz(props: VisualizationProps) { ]), filteredCounts: filteredCounts.value, xAxisVariable, + filters, + providedOverlayVariable, }; const data = useCachedPromise(async (): Promise< @@ -624,8 +626,13 @@ function HistogramViz(props: VisualizationProps) { > => { if (!dataRequestProps) throw new Error('dataRequestProps is not defined'); - const { dataRequestConfig, filteredCounts, xAxisVariable } = - dataRequestProps; + const { + dataRequestConfig, + filteredCounts, + xAxisVariable, + filters, + providedOverlayVariable, + } = dataRequestProps; if ( !variablesAreUnique([ From a6afabfc1478c98f4d6f3aeba3f1462232cd61e6 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 14:44:23 +0100 Subject: [PATCH 12/26] rename to dataRequestDeps and document it --- .../implementations/HistogramVisualization.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 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 cf09dbc7ef..b9a16bd479 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 @@ -594,12 +594,15 @@ function HistogramViz(props: VisualizationProps) { vizConfig.independentAxisValueSpec ); - const dataRequestProps = + // Serialisable values which will trigger a new data request when they change. + // Object will be undefined if certain critical things are missing. + // When undefined, no data request will be made. + const dataRequestDeps = vizConfig.xAxisVariable == null || xAxisVariable == null || filteredCounts.pending || filteredCounts.value == null - ? undefined // no data request will be made if undefined + ? undefined : { // pick only the props that should affect data requests // e.g. changes in dependentAxisLogScale should NOT trigger new data @@ -624,7 +627,7 @@ function HistogramViz(props: VisualizationProps) { const data = useCachedPromise(async (): Promise< HistogramDataWithCoverageStatistics | undefined > => { - if (!dataRequestProps) throw new Error('dataRequestProps is not defined'); + if (!dataRequestDeps) throw new Error('dataRequestDeps is not defined'); const { dataRequestConfig, @@ -632,7 +635,7 @@ function HistogramViz(props: VisualizationProps) { xAxisVariable, filters, providedOverlayVariable, - } = dataRequestProps; + } = dataRequestDeps; if ( !variablesAreUnique([ @@ -712,7 +715,7 @@ function HistogramViz(props: VisualizationProps) { ), showMissingOverlay ); - }, [dataRequestProps]); + }, [dataRequestDeps]); const [checkData, isEmptyData] = useMemo(() => { // controls need the bin info from just one facet (not an empty one) From 5fd09c82d568543a4f1ba6ea9babd543666d03a6 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 16:34:38 +0100 Subject: [PATCH 13/26] done barplot --- .../implementations/BarplotVisualization.tsx | 247 +++++++++--------- 1 file changed, 126 insertions(+), 121 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx index 4085f20803..cd4a0de866 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx @@ -20,7 +20,6 @@ import DataClient, { BarplotRequestParams, } from '../../../api/DataClient'; -import { usePromise } from '../../../hooks/promise'; import { useUpdateThumbnailEffect } from '../../../hooks/thumbnails'; import { useDataClient, @@ -105,6 +104,7 @@ import { useDeepValue } from '../../../hooks/immutability'; // reset to defaults button import { ResetButtonCoreUI } from '../../ResetButton'; import { useOutputEntity } from '../../../hooks/findOutputEntity'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; // export export type BarplotDataWithStatistics = ( @@ -381,136 +381,141 @@ function BarplotViz(props: VisualizationProps) { [options, providedOverlayVariable, providedOverlayVariableDescriptor] ); - const dataRequestConfig: DataRequestConfig = useDeepValue( - pick(vizConfig, [ - 'xAxisVariable', - 'overlayVariable', - 'facetVariable', - 'valueSpec', - 'showMissingness', - ]) - ); - - const data = usePromise( - useCallback(async (): Promise => { - if ( - variable == null || - filteredCounts.pending || - filteredCounts.value == null - ) - return undefined; - - if ( - !variablesAreUnique([ + // See HistogramVisualization for explanation + const dataRequestDeps = + variable == null || filteredCounts.pending || filteredCounts.value == null + ? undefined + : { + dataRequestConfig: pick(vizConfig, [ + 'xAxisVariable', + 'overlayVariable', + 'facetVariable', + 'valueSpec', + 'showMissingness', + ]), + filteredCounts: filteredCounts.value, variable, - overlayVariable && (providedOverlayVariable ?? overlayVariable), - facetVariable, - ]) - ) - throw new Error(nonUniqueWarning); - - assertValidInputVariables( - inputs, - selectedVariables, - entities, - dataElementConstraints, - dataElementDependencyOrder - ); - - if (outputEntity == null) return undefined; - - const params = - options?.getRequestParams?.({ - studyId, filters, - vizConfig: dataRequestConfig, - outputEntityId: outputEntity.id, - }) ?? - getRequestParams( - studyId, - filters ?? [], - dataRequestConfig, - outputEntity - ); - - const response = await dataClient.getBarplot( - computation.descriptor.type, - params - ); + providedOverlayVariable, + }; - // figure out if we need to show the missing data for the stratification variables - // if it has no incomplete cases we don't have to - const showMissingOverlay = - dataRequestConfig.showMissingness && - hasIncompleteCases( - overlayEntity, - overlayVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - const showMissingFacet = - dataRequestConfig.showMissingness && - hasIncompleteCases( - facetEntity, - facetVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - - const vocabulary = fixLabelsForNumberVariables( - variable?.vocabulary, - variable - ); - const overlayVocabulary = - (overlayVariable && options?.getOverlayVocabulary?.()) ?? - fixLabelsForNumberVariables( - overlayVariable?.vocabulary, - overlayVariable - ); - const facetVocabulary = fixLabelsForNumberVariables( - facetVariable?.vocabulary, - facetVariable - ); + const data = useCachedPromise(async (): Promise< + BarplotDataWithStatistics | undefined + > => { + if (!dataRequestDeps) throw new Error('dataRequestDeps not defined'); - return grayOutLastSeries( - substituteUnselectedToken( - reorderData( - barplotResponseToData( - response, - variable, - overlayVariable, - facetVariable - ), - vocabulary, - vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), - vocabularyWithMissingData(facetVocabulary, showMissingFacet) - ) - ), - showMissingOverlay - ); - }, [ + const { + dataRequestConfig, + filteredCounts, variable, - outputEntity, - filteredCounts.pending, - filteredCounts.value, - overlayVariable, - facetVariable, + filters, + providedOverlayVariable, + } = dataRequestDeps; + + if ( + !variablesAreUnique([ + variable, + overlayVariable && (providedOverlayVariable ?? overlayVariable), + facetVariable, + ]) + ) + throw new Error(nonUniqueWarning); + + assertValidInputVariables( inputs, selectedVariables, entities, dataElementConstraints, - dataElementDependencyOrder, - filters, - studyId, - dataRequestConfig, - dataClient, + dataElementDependencyOrder + ); + + if (outputEntity == null) return undefined; + + const params = + options?.getRequestParams?.({ + studyId, + filters, + vizConfig: dataRequestConfig, + outputEntityId: outputEntity.id, + }) ?? + getRequestParams(studyId, filters ?? [], dataRequestConfig, outputEntity); + + const response = await dataClient.getBarplot( computation.descriptor.type, - overlayEntity, - facetEntity, - ]) - ); + params + ); + + // figure out if we need to show the missing data for the stratification variables + // if it has no incomplete cases we don't have to + const showMissingOverlay = + dataRequestConfig.showMissingness && + hasIncompleteCases( + overlayEntity, + overlayVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + const showMissingFacet = + dataRequestConfig.showMissingness && + hasIncompleteCases( + facetEntity, + facetVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + + const vocabulary = fixLabelsForNumberVariables( + variable?.vocabulary, + variable + ); + const overlayVocabulary = + (overlayVariable && options?.getOverlayVocabulary?.()) ?? + fixLabelsForNumberVariables(overlayVariable?.vocabulary, overlayVariable); + const facetVocabulary = fixLabelsForNumberVariables( + facetVariable?.vocabulary, + facetVariable + ); + + return grayOutLastSeries( + substituteUnselectedToken( + reorderData( + barplotResponseToData( + response, + variable, + overlayVariable, + facetVariable + ), + vocabulary, + vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), + vocabularyWithMissingData(facetVocabulary, showMissingFacet) + ) + ), + showMissingOverlay + ); + }, [dataRequestDeps]); + + // variable, + // outputEntity, + // filteredCounts.pending, + // filteredCounts.value, + // overlayVariable, + // facetVariable, + // inputs, + // selectedVariables, + // entities, + // dataElementConstraints, + // dataElementDependencyOrder, + // filters, + // studyId, + // dataRequestConfig, + // dataClient, + // computation.descriptor.type, + // overlayEntity, + // facetEntity, + // ] + // ); const outputSize = (overlayVariable != null || facetVariable != null) && From b782daf9949d0318f1dc5390f6790593626c5d6f Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 16:35:22 +0100 Subject: [PATCH 14/26] remove commented deps --- .../implementations/BarplotVisualization.tsx | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx index cd4a0de866..21abc640c5 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx @@ -496,27 +496,6 @@ function BarplotViz(props: VisualizationProps) { ); }, [dataRequestDeps]); - // variable, - // outputEntity, - // filteredCounts.pending, - // filteredCounts.value, - // overlayVariable, - // facetVariable, - // inputs, - // selectedVariables, - // entities, - // dataElementConstraints, - // dataElementDependencyOrder, - // filters, - // studyId, - // dataRequestConfig, - // dataClient, - // computation.descriptor.type, - // overlayEntity, - // facetEntity, - // ] - // ); - const outputSize = (overlayVariable != null || facetVariable != null) && !vizConfig.showMissingness From a098e433459cc64111a70c71e25435f96c882ee4 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 22:49:37 +0100 Subject: [PATCH 15/26] standardise throw messages --- .../visualizations/implementations/BarplotVisualization.tsx | 2 +- .../visualizations/implementations/HistogramVisualization.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx index 21abc640c5..6bbb9b7dfe 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx @@ -429,7 +429,7 @@ function BarplotViz(props: VisualizationProps) { dataElementDependencyOrder ); - if (outputEntity == null) return undefined; + if (outputEntity == null) throw new Error('outputEntity is undefined'); const params = options?.getRequestParams?.({ 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 b9a16bd479..3a5941e306 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 @@ -627,7 +627,7 @@ function HistogramViz(props: VisualizationProps) { const data = useCachedPromise(async (): Promise< HistogramDataWithCoverageStatistics | undefined > => { - if (!dataRequestDeps) throw new Error('dataRequestDeps is not defined'); + if (!dataRequestDeps) throw new Error('dataRequestDeps is undefined'); const { dataRequestConfig, From 12d43bd67c92349354666e62246528da80a92c6e Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 18 Oct 2024 22:50:17 +0100 Subject: [PATCH 16/26] boxplot --- .../implementations/BoxplotVisualization.tsx | 281 ++++++++---------- 1 file changed, 130 insertions(+), 151 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx index ab50248752..71487e5f1a 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx @@ -8,7 +8,6 @@ import { useCallback, useMemo, useState, useEffect } from 'react'; // need to set for Boxplot import DataClient, { BoxplotResponse } from '../../../api/DataClient'; -import { usePromise } from '../../../hooks/promise'; import { useFindEntityAndVariable, useStudyEntities, @@ -111,6 +110,7 @@ import { useDeepValue } from '../../../hooks/immutability'; // reset to defaults button import { ResetButtonCoreUI } from '../../ResetButton'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; type BoxplotData = { series: BoxplotSeries }; // type of computedVariableMetadata for computation apps such as alphadiv and abundance @@ -430,165 +430,144 @@ function BoxplotViz(props: VisualizationProps) { ] ); - // add to support both alphadiv and abundance - const data = usePromise( - useCallback(async (): Promise => { - if ( - // check for vizConfig variables only if provided variables are not defined. - (providedXAxisVariable == null && - (vizConfig.xAxisVariable == null || xAxisVariable == null)) || - (computedYAxisDetails == null && - (vizConfig.yAxisVariable == null || yAxisVariable == null)) || - filteredCounts.pending || - filteredCounts.value == null - ) - return undefined; - - // If this boxplot has a computed variable and the compute job is anything but complete, do not proceed with getting data. - if (computedYAxisDetails && computeJobStatus !== 'complete') - return undefined; - - if ( - !variablesAreUnique([ - xAxisVariable, - yAxisVariable, - overlayVariable && (providedOverlayVariable ?? overlayVariable), - facetVariable, - ]) - ) - throw new Error(nonUniqueWarning); - - assertValidInputVariables( - inputs, - variablesForConstraints, - entities, - dataElementConstraints, - dataElementDependencyOrder - ); - - if (outputEntity == null) return undefined; - - // add visualization.type here. valueSpec too? - const params: BoxplotRequestParams = options?.getRequestParams?.({ - studyId, - filters: filters ?? [], - vizConfig, - outputEntityId: outputEntity.id, - computation, - }) ?? { - studyId, - filters: filters ?? [], - config: { - outputEntityId: outputEntity.id, - // post options: 'all', 'outliers' - points: 'outliers', - mean: 'TRUE', - xAxisVariable: vizConfig.xAxisVariable, - yAxisVariable: vizConfig.yAxisVariable, - overlayVariable: vizConfig.overlayVariable, - facetVariable: vizConfig.facetVariable - ? [vizConfig.facetVariable] - : [], - showMissingness: vizConfig.showMissingness ? 'TRUE' : 'FALSE', - }, - computeConfig: copmutationAppOverview.computeName - ? computation.descriptor.configuration - : undefined, - }; + const dataRequestDeps = + // check for vizConfig variables only if provided variables are not defined. + (providedXAxisVariable == null && + (vizConfig.xAxisVariable == null || xAxisVariable == null)) || + (computedYAxisDetails == null && + (vizConfig.yAxisVariable == null || yAxisVariable == null)) || + filteredCounts.pending || + filteredCounts.value == null || + // If this boxplot has a computed variable and the compute job is anything but complete, do not proceed with getting data. + (computedYAxisDetails && computeJobStatus !== 'complete') + ? undefined + : { + vizConfig, + filteredCounts: filteredCounts.value, + filters, + providedOverlayVariable, + computation, + }; - // 2024-04-26 - BM wonders why we don't use getBoxplot? - // or why we don't just use this for all the visualizations? - const response = await dataClient.getVisualizationData( - computation.descriptor.type, - visualization.descriptor.type, - params, - BoxplotResponse - ); + // add to support both alphadiv and abundance + const data = useCachedPromise(async (): Promise< + BoxplotDataWithCoverage | undefined + > => { + if (!dataRequestDeps) throw new Error('dataRequestDeps is not defined'); + + const { + vizConfig, + filteredCounts, + filters, + providedOverlayVariable, + computation, + } = dataRequestDeps; - const showMissingOverlay = - vizConfig.showMissingness && - hasIncompleteCases( - overlayEntity, - overlayVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - const showMissingFacet = - vizConfig.showMissingness && - hasIncompleteCases( - facetEntity, - facetVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); + if ( + !variablesAreUnique([ + xAxisVariable, + yAxisVariable, + overlayVariable && (providedOverlayVariable ?? overlayVariable), + facetVariable, + ]) + ) + throw new Error(nonUniqueWarning); - const vocabulary = fixLabelsForNumberVariables( - xAxisVariable?.vocabulary, - xAxisVariable - ); - const overlayVocabulary = - (overlayVariable && options?.getOverlayVocabulary?.()) ?? - fixLabelsForNumberVariables( - overlayVariable?.vocabulary, - overlayVariable - ); - const facetVocabulary = fixLabelsForNumberVariables( - facetVariable?.vocabulary, - facetVariable - ); - return grayOutLastSeries( - substituteUnselectedToken( - reorderData( - boxplotResponseToData( - response, - xAxisVariable, - overlayVariable, - facetVariable, - entities - ), - vocabulary, - vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), - vocabularyWithMissingData(facetVocabulary, showMissingFacet), - entities - ) - ), - showMissingOverlay, - '#a0a0a0' - ); - }, [ - providedXAxisVariable, - vizConfig.xAxisVariable, - vizConfig.yAxisVariable, - vizConfig.overlayVariable, - vizConfig.facetVariable, - vizConfig.showMissingness, - xAxisVariable, - computedYAxisDetails, - yAxisVariable, - outputEntity, - filteredCounts.pending, - filteredCounts.value, - computeJobStatus, - overlayVariable, - facetVariable, + assertValidInputVariables( inputs, - selectedVariables, + variablesForConstraints, entities, dataElementConstraints, - dataElementDependencyOrder, - filters, + dataElementDependencyOrder + ); + + if (outputEntity == null) throw new Error('outputEntity is undefined'); + + // add visualization.type here. valueSpec too? + const params: BoxplotRequestParams = options?.getRequestParams?.({ + studyId, + filters: filters ?? [], + vizConfig, + outputEntityId: outputEntity.id, + computation, + }) ?? { studyId, - computation.descriptor.configuration, + filters: filters ?? [], + config: { + outputEntityId: outputEntity.id, + // post options: 'all', 'outliers' + points: 'outliers', + mean: 'TRUE', + xAxisVariable: vizConfig.xAxisVariable, + yAxisVariable: vizConfig.yAxisVariable, + overlayVariable: vizConfig.overlayVariable, + facetVariable: vizConfig.facetVariable ? [vizConfig.facetVariable] : [], + showMissingness: vizConfig.showMissingness ? 'TRUE' : 'FALSE', + }, + computeConfig: copmutationAppOverview.computeName + ? computation.descriptor.configuration + : undefined, + }; + + // 2024-04-26 - BM wonders why we don't use getBoxplot? + // or why we don't just use this for all the visualizations? + const response = await dataClient.getVisualizationData( computation.descriptor.type, - dataClient, visualization.descriptor.type, - overlayEntity, - facetEntity, - variablesForConstraints, - ]) - ); + params, + BoxplotResponse + ); + + const showMissingOverlay = + vizConfig.showMissingness && + hasIncompleteCases( + overlayEntity, + overlayVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + const showMissingFacet = + vizConfig.showMissingness && + hasIncompleteCases( + facetEntity, + facetVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + + const vocabulary = fixLabelsForNumberVariables( + xAxisVariable?.vocabulary, + xAxisVariable + ); + const overlayVocabulary = + (overlayVariable && options?.getOverlayVocabulary?.()) ?? + fixLabelsForNumberVariables(overlayVariable?.vocabulary, overlayVariable); + const facetVocabulary = fixLabelsForNumberVariables( + facetVariable?.vocabulary, + facetVariable + ); + return grayOutLastSeries( + substituteUnselectedToken( + reorderData( + boxplotResponseToData( + response, + xAxisVariable, + overlayVariable, + facetVariable, + entities + ), + vocabulary, + vocabularyWithMissingData(overlayVocabulary, showMissingOverlay), + vocabularyWithMissingData(facetVocabulary, showMissingFacet), + entities + ) + ), + showMissingOverlay, + '#a0a0a0' + ); + }, [dataRequestDeps]); const outputSize = (overlayVariable != null || facetVariable != null) && From db32ab4fbf9cfc74b9d0780d3d3988b477b30660 Mon Sep 17 00:00:00 2001 From: Bob Date: Sat, 19 Oct 2024 12:12:44 +0100 Subject: [PATCH 17/26] fix mbio boxplot behaviour --- .../implementations/BoxplotVisualization.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx index 71487e5f1a..a3077b7264 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/BoxplotVisualization.tsx @@ -446,7 +446,7 @@ function BoxplotViz(props: VisualizationProps) { filteredCounts: filteredCounts.value, filters, providedOverlayVariable, - computation, + computationDescriptor: computation.descriptor, // computation.visualizations is not stable }; // add to support both alphadiv and abundance @@ -460,7 +460,7 @@ function BoxplotViz(props: VisualizationProps) { filteredCounts, filters, providedOverlayVariable, - computation, + computationDescriptor, } = dataRequestDeps; if ( @@ -505,14 +505,14 @@ function BoxplotViz(props: VisualizationProps) { showMissingness: vizConfig.showMissingness ? 'TRUE' : 'FALSE', }, computeConfig: copmutationAppOverview.computeName - ? computation.descriptor.configuration + ? computationDescriptor.configuration : undefined, }; // 2024-04-26 - BM wonders why we don't use getBoxplot? // or why we don't just use this for all the visualizations? const response = await dataClient.getVisualizationData( - computation.descriptor.type, + computationDescriptor.type, visualization.descriptor.type, params, BoxplotResponse From 541b59a56edbee32eed4560a26ab4370ab930a02 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Mon, 21 Oct 2024 12:32:03 +0100 Subject: [PATCH 18/26] started with lineplot --- .../implementations/LineplotVisualization.tsx | 302 +++++++++--------- 1 file changed, 153 insertions(+), 149 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx index c455609611..aa2b482904 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx @@ -11,7 +11,7 @@ import DataClient, { LineplotResponse, } from '../../../api/DataClient'; -import { usePromise, PromiseHookState } from '../../../hooks/promise'; +import { PromiseHookState } from '../../../hooks/promise'; import { useUpdateThumbnailEffect } from '../../../hooks/thumbnails'; import { useDataClient, @@ -142,6 +142,7 @@ import { invalidProportionText, validateProportionValues, } from '../../../../map/analysis/utils/defaultOverlayConfig'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; const plotContainerStyles = { width: 750, @@ -692,164 +693,175 @@ function LineplotViz(props: VisualizationProps) { const showDependentAxisBanner = vizConfig.dependentAxisLogScale && vizConfig.showErrorBars; - const data = usePromise( - useCallback(async (): Promise => { - if ( - xAxisVariable == null || - yAxisVariable == null || - filteredCounts.pending || - filteredCounts.value == null - ) - return undefined; + const varsAreUnique = variablesAreUnique([ + xAxisVariable, + yAxisVariable, + overlayVariable && (providedOverlayVariable ?? overlayVariable), + facetVariable, + ]); - if ( - !variablesAreUnique([ + const dataRequestDeps = + xAxisVariable == null || + yAxisVariable == null || + filteredCounts.pending || + filteredCounts.value == null || + (varsAreUnique && categoricalMode && !valuesAreSpecified) + ? undefined + : { + dataRequestConfig, xAxisVariable, yAxisVariable, - overlayVariable && (providedOverlayVariable ?? overlayVariable), - facetVariable, - ]) - ) - throw new Error(nonUniqueWarning); + filters, + filteredCounts: filteredCounts.value, + }; - if (categoricalMode && !valuesAreSpecified) return undefined; + const data = useCachedPromise(async (): Promise< + LinePlotDataWithCoverage | undefined + > => { + if (!dataRequestDeps) throw new Error('dataRequestDeps is undefined'); - if ( - categoricalMode && - !validateProportionValues( - dataRequestConfig.numeratorValues, - dataRequestConfig.denominatorValues, - yAxisVariable?.vocabulary - ) - ) - throw new Error(invalidProportionText); - - // no data request if banner should be shown - if (showIndependentAxisBanner || showDependentAxisBanner) - return undefined; - - assertValidInputVariables( - inputs, - selectedVariables, - entities, - dataElementConstraints, - dataElementDependencyOrder - ); - - if (outputEntity == null) return undefined; - - // check independentValueType/dependentValueType - const independentValueType = xAxisVariable?.type - ? xAxisVariable.type - : ''; - const dependentValueType = yAxisVariable?.type ? yAxisVariable.type : ''; - - // add visualization.type here. valueSpec too? - const params = getRequestParams( - studyId, - filters ?? [], - dataRequestConfig, - xAxisVariable, - yAxisVariable, - outputEntity, - options?.getRequestParams - ); + const { + dataRequestConfig, + xAxisVariable, + yAxisVariable, + filters, + filteredCounts, + } = dataRequestDeps; - const response = await dataClient.getLineplot( - computation.descriptor.type, - visualization.descriptor.type, - params - ); + if (!variablesAreUnique) throw new Error(nonUniqueWarning); - const showMissingOverlay = - dataRequestConfig.showMissingness && - hasIncompleteCases( - overlayEntity, - overlayVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); - const showMissingFacet = - dataRequestConfig.showMissingness && - hasIncompleteCases( - facetEntity, - facetVariable, - outputEntity, - filteredCounts.value, - response.completeCasesTable - ); + if ( + categoricalMode && + !validateProportionValues( + dataRequestConfig.numeratorValues, + dataRequestConfig.denominatorValues, + yAxisVariable?.vocabulary + ) + ) + throw new Error(invalidProportionText); - // This is used for reordering series data. - // We must not reorder binned data from continous vars (number and date) - // because then the data gets "lost" - nothing plotted. - // Also integer ordinals get binned too - so we disable the vocabulary for them. - const xAxisVocabulary = - xAxisVariable.dataShape === 'continuous' || - (xAxisVariable.dataShape === 'ordinal' && - xAxisVariable.type === 'integer') - ? [] - : fixLabelsForNumberVariables( - xAxisVariable?.vocabulary, - xAxisVariable - ); - const overlayVocabulary = - (overlayVariable && options?.getOverlayVocabulary?.()) ?? - fixLabelsForNumberVariables( - overlayVariable?.vocabulary, - overlayVariable - ); - const facetVocabulary = fixLabelsForNumberVariables( - facetVariable?.vocabulary, - facetVariable + // no data request if banner should be shown + if (showIndependentAxisBanner || showDependentAxisBanner) + throw new Error( + 'what are these showIndependentAxisBanner and showDependentAxisBanner all about??????' ); - return lineplotResponseToData( - response, - categoricalMode, - visualization.descriptor.type, - independentValueType, - dependentValueType, - params.config.valueSpec === 'proportion', - showMissingOverlay, - xAxisVocabulary, - overlayVocabulary, - overlayVariable, - showMissingFacet, - facetVocabulary, - facetVariable, - colorPaletteOverride, - // pass showmarginalHistogram - showMarginalHistogram - ); - }, [ - outputEntity, - xAxisVariable, - yAxisVariable, - filteredCounts.pending, - filteredCounts.value, - overlayVariable, - facetVariable, - categoricalMode, - valuesAreSpecified, + assertValidInputVariables( inputs, selectedVariables, entities, dataElementConstraints, - dataElementDependencyOrder, - filters, + dataElementDependencyOrder + ); + + if (outputEntity == null) throw new Error('outputEntity is undefined'); + + // check independentValueType/dependentValueType + const independentValueType = xAxisVariable?.type ? xAxisVariable.type : ''; + const dependentValueType = yAxisVariable?.type ? yAxisVariable.type : ''; + + // add visualization.type here. valueSpec too? + const params = getRequestParams( studyId, + filters ?? [], dataRequestConfig, - dataClient, + xAxisVariable, + yAxisVariable, + outputEntity, + options?.getRequestParams + ); + + const response = await dataClient.getLineplot( computation.descriptor.type, - overlayEntity, - facetEntity, visualization.descriptor.type, - neutralPaletteProps.colorPalette, - showIndependentAxisBanner, - showDependentAxisBanner, - ]) - ); + params + ); + + const showMissingOverlay = + dataRequestConfig.showMissingness && + hasIncompleteCases( + overlayEntity, + overlayVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + const showMissingFacet = + dataRequestConfig.showMissingness && + hasIncompleteCases( + facetEntity, + facetVariable, + outputEntity, + filteredCounts, + response.completeCasesTable + ); + + // This is used for reordering series data. + // We must not reorder binned data from continous vars (number and date) + // because then the data gets "lost" - nothing plotted. + // Also integer ordinals get binned too - so we disable the vocabulary for them. + const xAxisVocabulary = + xAxisVariable.dataShape === 'continuous' || + (xAxisVariable.dataShape === 'ordinal' && + xAxisVariable.type === 'integer') + ? [] + : fixLabelsForNumberVariables(xAxisVariable?.vocabulary, xAxisVariable); + const overlayVocabulary = + (overlayVariable && options?.getOverlayVocabulary?.()) ?? + fixLabelsForNumberVariables(overlayVariable?.vocabulary, overlayVariable); + const facetVocabulary = fixLabelsForNumberVariables( + facetVariable?.vocabulary, + facetVariable + ); + + return lineplotResponseToData( + response, + categoricalMode, + visualization.descriptor.type, + independentValueType, + dependentValueType, + params.config.valueSpec === 'proportion', + showMissingOverlay, + xAxisVocabulary, + overlayVocabulary, + overlayVariable, + showMissingFacet, + facetVocabulary, + facetVariable, + colorPaletteOverride, + // pass showmarginalHistogram + showMarginalHistogram + ); + }, [dataRequestDeps]); + + // [ + // outputEntity, + // xAxisVariable, + // yAxisVariable, + // filteredCounts.pending, + // filteredCounts.value, + // overlayVariable, + // facetVariable, + // categoricalMode, + // valuesAreSpecified, + // inputs, + // selectedVariables, + // entities, + // dataElementConstraints, + // dataElementDependencyOrder, + // filters, + // studyId, + // dataRequestConfig, + // dataClient, + // computation.descriptor.type, + // overlayEntity, + // facetEntity, + // visualization.descriptor.type, + // neutralPaletteProps.colorPalette, + // showIndependentAxisBanner, + // showDependentAxisBanner, + // ]) + // ); const outputSize = overlayVariable != null && !vizConfig.showMissingness @@ -2076,14 +2088,6 @@ type DataRequestConfig = Omit< 'dependentAxisRange' | 'checkedLegendItems' >; -/** - * Passing the whole of `vizConfig` creates a problem with the TypeScript compiler warnings - * for the dependencies of the `data = usePromise(...)` that calls this function. It warns - * that `vizConfig` is a missing dependency because it see it being used (passed to `getRequestParams()`) - * We can't use the whole of `vizConfig` as a dependency because then data will be re-requested - * when only client-side configs are changed. There should probably be two sub-objects of `vizConfig`, - * for client and server-side configs. - */ function getRequestParams( studyId: string, filters: Filter[], From b840c6e6436f2c22aea0fdff05497c39a4513704 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Mon, 21 Oct 2024 12:56:44 +0100 Subject: [PATCH 19/26] fix oopsie with unique vars check --- .../visualizations/implementations/LineplotVisualization.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx index aa2b482904..4493df904c 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx @@ -728,7 +728,7 @@ function LineplotViz(props: VisualizationProps) { filteredCounts, } = dataRequestDeps; - if (!variablesAreUnique) throw new Error(nonUniqueWarning); + if (!varsAreUnique) throw new Error(nonUniqueWarning); if ( categoricalMode && From 0e60f89540169cbcfed10f6e65f2313cecb37980 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Mon, 21 Oct 2024 13:05:09 +0100 Subject: [PATCH 20/26] fixed lineplot incompatible settings banner behaviour and used variables properly --- .../implementations/LineplotVisualization.tsx | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx index 4493df904c..34746361fd 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx @@ -705,7 +705,9 @@ function LineplotViz(props: VisualizationProps) { yAxisVariable == null || filteredCounts.pending || filteredCounts.value == null || - (varsAreUnique && categoricalMode && !valuesAreSpecified) + (varsAreUnique && categoricalMode && !valuesAreSpecified) || + showIndependentAxisBanner || + showDependentAxisBanner ? undefined : { dataRequestConfig, @@ -740,12 +742,6 @@ function LineplotViz(props: VisualizationProps) { ) throw new Error(invalidProportionText); - // no data request if banner should be shown - if (showIndependentAxisBanner || showDependentAxisBanner) - throw new Error( - 'what are these showIndependentAxisBanner and showDependentAxisBanner all about??????' - ); - assertValidInputVariables( inputs, selectedVariables, @@ -1296,7 +1292,7 @@ function LineplotViz(props: VisualizationProps) { }} > {/* independent axis banner */} - {vizConfig.independentAxisLogScale && vizConfig.useBinning && ( + {showIndependentAxisBanner && ( ) { /> )} {/* dependent axis banner */} - {vizConfig.dependentAxisLogScale && vizConfig.showErrorBars && ( + {showDependentAxisBanner && ( ) { > {independentAllNegative && !dismissedIndependentAllNegativeWarning && - !(vizConfig.independentAxisLogScale && vizConfig.useBinning) && - !(vizConfig.dependentAxisLogScale && vizConfig.showErrorBars) ? ( + !showIndependentAxisBanner && + !showDependentAxisBanner ? ( ) { {/* truncation notification */} {truncatedIndependentAxisWarning && !independentAllNegative && - !(vizConfig.independentAxisLogScale && vizConfig.useBinning) && - !(vizConfig.dependentAxisLogScale && vizConfig.showErrorBars) ? ( + !showIndependentAxisBanner && + !showDependentAxisBanner ? ( ) { > {dependentAllNegative && !dismissedDependentAllNegativeWarning && - !(vizConfig.independentAxisLogScale && vizConfig.useBinning) && - !(vizConfig.dependentAxisLogScale && vizConfig.showErrorBars) ? ( + !showIndependentAxisBanner && + !showDependentAxisBanner ? ( ) { {/* truncation notification */} {truncatedDependentAxisWarning && !dependentAllNegative && - !(vizConfig.independentAxisLogScale && vizConfig.useBinning) && - !(vizConfig.dependentAxisLogScale && vizConfig.showErrorBars) ? ( + !showIndependentAxisBanner && + !showDependentAxisBanner ? ( Date: Mon, 21 Oct 2024 13:29:29 +0100 Subject: [PATCH 21/26] take care of floating plot overlay and remove commented deps --- .../implementations/LineplotVisualization.tsx | 32 ++----------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx index 34746361fd..af90bd616a 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx @@ -713,6 +713,7 @@ function LineplotViz(props: VisualizationProps) { dataRequestConfig, xAxisVariable, yAxisVariable, + inputs, // includes providedOverlayVariable filters, filteredCounts: filteredCounts.value, }; @@ -726,6 +727,7 @@ function LineplotViz(props: VisualizationProps) { dataRequestConfig, xAxisVariable, yAxisVariable, + inputs, filters, filteredCounts, } = dataRequestDeps; @@ -825,40 +827,10 @@ function LineplotViz(props: VisualizationProps) { facetVocabulary, facetVariable, colorPaletteOverride, - // pass showmarginalHistogram showMarginalHistogram ); }, [dataRequestDeps]); - // [ - // outputEntity, - // xAxisVariable, - // yAxisVariable, - // filteredCounts.pending, - // filteredCounts.value, - // overlayVariable, - // facetVariable, - // categoricalMode, - // valuesAreSpecified, - // inputs, - // selectedVariables, - // entities, - // dataElementConstraints, - // dataElementDependencyOrder, - // filters, - // studyId, - // dataRequestConfig, - // dataClient, - // computation.descriptor.type, - // overlayEntity, - // facetEntity, - // visualization.descriptor.type, - // neutralPaletteProps.colorPalette, - // showIndependentAxisBanner, - // showDependentAxisBanner, - // ]) - // ); - const outputSize = overlayVariable != null && !vizConfig.showMissingness ? data.value?.completeCasesAllVars From 9994ca2f43f1874413dfe6ba60d9a4f185a95f8b Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 25 Oct 2024 16:04:47 +0100 Subject: [PATCH 22/26] mosaics done --- .../implementations/MosaicVisualization.tsx | 198 +++++++----------- 1 file changed, 77 insertions(+), 121 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/MosaicVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/MosaicVisualization.tsx index 3feae3a34d..359354fad3 100644 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/MosaicVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/MosaicVisualization.tsx @@ -19,7 +19,6 @@ import DataClient, { facetVariableDetailsType, } from '../../../api/DataClient'; import { useCallback, useMemo, useState } from 'react'; -import { usePromise } from '../../../hooks/promise'; import { useUpdateThumbnailEffect } from '../../../hooks/thumbnails'; import { useDataClient, @@ -71,6 +70,7 @@ import { useInputStyles } from '../inputStyles'; import { ClearSelectionButton } from '../../variableSelectors/VariableTreeDropdown'; import { Tooltip } from '@veupathdb/coreui'; import Banner from '@veupathdb/coreui/lib/components/banners/Banner'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; /** * Note: When options.hideFacetInputs is true, the mosaic plot is not shown. @@ -377,135 +377,87 @@ function MosaicViz(props: Props) { [isTwoByTwo, options?.hideFacetInputs] ); - const data = usePromise( - useCallback(async (): Promise< - TwoByTwoDataWithCoverage | ContTableDataWithCoverage | undefined - > => { - if ( - vizConfig.xAxisVariable == null || - xAxisVariable == null || - vizConfig.yAxisVariable == null || - yAxisVariable == null - ) - return undefined; - - if (!variablesAreUnique([xAxisVariable, yAxisVariable, facetVariable])) - throw new Error(nonUniqueWarning); - - assertValidInputVariables( - inputs, - selectedVariables, - entities, - dataElementConstraints - ); - - if (outputEntity == null) return undefined; - - const xAxisVocabulary = fixLabelsForNumberVariables( - xAxisVariable.vocabulary, - xAxisVariable - ); - const yAxisVocabulary = fixLabelsForNumberVariables( - yAxisVariable.vocabulary, - yAxisVariable - ); - const facetVocabulary = fixLabelsForNumberVariables( - facetVariable?.vocabulary, - facetVariable - ); - - if (isTwoByTwo) { - if ( - !vizConfig.xAxisReferenceValue || - !xAxisReferenceValue || - !vizConfig.yAxisReferenceValue || - !yAxisReferenceValue - ) - return undefined; - - const params = getRequestParams( - studyId, - filters ?? [], - vizConfig.xAxisVariable, - vizConfig.yAxisVariable, - outputEntity.id, - vizConfig.facetVariable, - vizConfig.showMissingness, - vizConfig.xAxisReferenceValue, - vizConfig.yAxisReferenceValue, - options - ); + const dataRequestDeps = + vizConfig.xAxisVariable == null || + xAxisVariable == null || + vizConfig.yAxisVariable == null || + yAxisVariable == null || + (isTwoByTwo && (xAxisReferenceValue == null || yAxisReferenceValue == null)) + ? undefined + : { + xAxisVariable, + yAxisVariable, + vizConfig, + }; - const response = dataClient.getTwoByTwo( - computation.descriptor.type, - params - ); + const data = useCachedPromise(async (): Promise< + TwoByTwoDataWithCoverage | ContTableDataWithCoverage | undefined + > => { + if (!dataRequestDeps) throw new Error('dataRequestDeps is undefined'); + + const { xAxisVariable, yAxisVariable, vizConfig } = dataRequestDeps; + + if (!variablesAreUnique([xAxisVariable, yAxisVariable, facetVariable])) + throw new Error(nonUniqueWarning); + + assertValidInputVariables( + inputs, + selectedVariables, + entities, + dataElementConstraints + ); + + if (outputEntity == null) throw new Error('outputEntity is undefined'); + + const xAxisVocabulary = fixLabelsForNumberVariables( + xAxisVariable.vocabulary, + xAxisVariable + ); + const yAxisVocabulary = fixLabelsForNumberVariables( + yAxisVariable.vocabulary, + yAxisVariable + ); + const facetVocabulary = fixLabelsForNumberVariables( + facetVariable?.vocabulary, + facetVariable + ); + + const params = getRequestParams( + studyId, + filters ?? [], + vizConfig.xAxisVariable!, + vizConfig.yAxisVariable!, + outputEntity.id, + vizConfig.facetVariable, + vizConfig.showMissingness, + xAxisReferenceValue, // always undefined + yAxisReferenceValue, // in twoByTwo mode + options + ); - return reorderData( - twoByTwoResponseToData( + const response = isTwoByTwo + ? dataClient.getTwoByTwo(computation.descriptor.type, params) + : dataClient.getContTable(computation.descriptor.type, params); + + return reorderData( + isTwoByTwo + ? twoByTwoResponseToData( await response, xAxisVariable, yAxisVariable, facetVariable - ), - xAxisVocabulary, - yAxisVocabulary, - vocabularyWithMissingData(facetVocabulary, vizConfig.showMissingness) - ) as TwoByTwoDataWithCoverage; - } else { - const params = getRequestParams( - studyId, - filters ?? [], - vizConfig.xAxisVariable, - vizConfig.yAxisVariable, - outputEntity?.id ?? '', - vizConfig.facetVariable, - vizConfig.showMissingness, - undefined, - undefined, - options - ); - const response = dataClient.getContTable( - computation.descriptor.type, - params - ); - - return reorderData( - contTableResponseToData( + ) + : contTableResponseToData( await response, xAxisVariable, yAxisVariable, facetVariable ), - xAxisVocabulary, - yAxisVocabulary, - vocabularyWithMissingData(facetVocabulary, vizConfig.showMissingness) - ) as ContTableDataWithCoverage; - } - }, [ - vizConfig.xAxisVariable, - vizConfig.yAxisVariable, - vizConfig.xAxisReferenceValue, - vizConfig.yAxisReferenceValue, - vizConfig.facetVariable, - vizConfig.showMissingness, - xAxisVariable, - yAxisVariable, - facetVariable, - inputs, - selectedVariables, - entities, - dataElementConstraints, - filters, - isTwoByTwo, - xAxisReferenceValue, - yAxisReferenceValue, - studyId, - outputEntity?.id, - dataClient, - computation.descriptor.type, - ]) - ); + xAxisVocabulary, + yAxisVocabulary, + vocabularyWithMissingData(facetVocabulary, vizConfig.showMissingness) + ) as TwoByTwoDataWithCoverage | ContTableDataWithCoverage; + }, [dataRequestDeps]); const xAxisLabel = variableDisplayWithUnit(xAxisVariable); const yAxisLabel = variableDisplayWithUnit(yAxisVariable); @@ -1626,8 +1578,12 @@ function ContTableStats(props?: { * Reformat response from mosaic endpoints into complete MosaicData * @param response * @returns MosaicData + * + * NOTE: this and twoByTwoResponseToData ought to be refactored into one function + * because they share a lot of logic. However the TypeScript seems tricky, so we haven't so far. + * */ -export function contTableResponseToData( +function contTableResponseToData( response: ContTableResponse, xVariable: Variable, yVariable: Variable, @@ -1706,7 +1662,7 @@ export function contTableResponseToData( * @param response * @returns MosaicData */ -export function twoByTwoResponseToData( +function twoByTwoResponseToData( response: TwoByTwoResponse, xVariable: Variable, yVariable: Variable, From f66f3c16df0e68d47900aa98fec9ecdcfd0d76cb Mon Sep 17 00:00:00 2001 From: Bob Date: Sat, 26 Oct 2024 15:11:51 +0100 Subject: [PATCH 23/26] scatterplot --- .../ScatterplotVisualization.tsx | 139 ++++++++---------- 1 file changed, 64 insertions(+), 75 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/ScatterplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/ScatterplotVisualization.tsx index f774c02fa8..aa009502fa 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/ScatterplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/ScatterplotVisualization.tsx @@ -10,7 +10,6 @@ import { useCallback, useMemo, useState, useEffect } from 'react'; import DataClient, { ScatterplotResponse } from '../../../api/DataClient'; -import { usePromise } from '../../../hooks/promise'; import { useUpdateThumbnailEffect } from '../../../hooks/thumbnails'; import { useDataClient, @@ -60,6 +59,7 @@ import { uniqBy, filter, isEqual, + omit, } from 'lodash'; // directly use RadioButtonGroup instead of ScatterPlotControls import RadioButtonGroup from '@veupathdb/components/lib/components/widgets/RadioButtonGroup'; @@ -149,6 +149,7 @@ import SliderWidget, { import { FloatingScatterplotExtraProps } from '../../../../map/analysis/hooks/plugins/scatterplot'; import { Override } from '../../../types/utility'; +import { useCachedPromise } from '../../../hooks/cachedPromise'; const MAXALLOWEDDATAPOINTS = 100000; const SMOOTHEDMEANTEXT = 'Smoothed mean'; @@ -644,14 +645,58 @@ function ScatterplotViz(props: VisualizationProps) { [] ); - const data = usePromise( - useCallback(async (): Promise => { - // If this scatterplot has a computed variable and the compute job is anything but complete, do not proceed with getting data. - if (computedYAxisDetails && computeJobStatus !== 'complete') - return undefined; + const dataRequestDeps = + // If this scatterplot has a computed variable and the compute job is anything but complete, do not proceed with getting data. + (computedYAxisDetails && computeJobStatus !== 'complete') || + // the usual conditions for not showing a plot: + filteredCounts.pending || + filteredCounts.value == null || + showLogScaleBanner || + showContinousOverlayBanner || + // check for required variables when not a compute + (computedXAxisDetails == null && + (vizConfig.xAxisVariable == null || xAxisVariable == null)) || + (computedYAxisDetails == null && + (vizConfig.yAxisVariable == null || yAxisVariable == null)) + ? undefined + : { + vizConfig: omit( + // same as additional dependencies to useUpdateThumbnailEffect + vizConfig, + [ + 'checkedLegendItems', + 'independentAxisRange', + 'dependentAxisRange', + 'independentAxisLogScale', + 'dependentAxisLogScale', + 'independentAxisValueSpec', + 'dependentAxisValueSpec', + 'markerBodyOpacity', + ] + ), + filteredCounts: filteredCounts.value, + providedOverlayVariable, + computationDescriptor: computation.descriptor, + hideTrendlines: options?.hideTrendlines, + overlayMin, + overlayMax, + computedOverlayVariableDescriptor, + }; - if (filteredCounts.pending || filteredCounts.value == null) - return undefined; + const data = useCachedPromise( + async (): Promise => { + if (!dataRequestDeps) throw new Error('dataRequestDeps is undefined'); + + const { + vizConfig, + filteredCounts, + providedOverlayVariable, + computationDescriptor, + hideTrendlines, + overlayMin, + overlayMax, + computedOverlayVariableDescriptor, + } = dataRequestDeps; if ( !variablesAreUnique([ @@ -671,27 +716,7 @@ function ScatterplotViz(props: VisualizationProps) { dataElementDependencyOrder ); - if (outputEntity == null) return undefined; - - // check log scale and plot mode option for retrieving data - if (showLogScaleBanner) return undefined; - - // check variable inputs: this is necessary to prevent from data post - if ( - computedXAxisDetails == null && - (vizConfig.xAxisVariable == null || xAxisVariable == null) - ) - return undefined; - else if ( - computedYAxisDetails == null && - (vizConfig.yAxisVariable == null || yAxisVariable == null) - ) - return undefined; - - // prevent data request for the case of plot option != Raw when using continuous overlayVariable - if (showContinousOverlayBanner) { - return undefined; - } + if (outputEntity == null) throw new Error('outputEntity is undefined'); // Convert valueSpecConfig to valueSpecValue for the data client request. let valueSpecValue: ScatterplotRequestParams['config']['valueSpec'] = @@ -708,13 +733,13 @@ function ScatterplotViz(props: VisualizationProps) { filters, outputEntityId: outputEntity.id, vizConfig, - valueSpec: options?.hideTrendlines ? undefined : valueSpecValue, + valueSpec: hideTrendlines ? undefined : valueSpecValue, }) ?? { studyId, filters, config: { outputEntityId: outputEntity.id, - valueSpec: options?.hideTrendlines ? undefined : valueSpecValue, + valueSpec: hideTrendlines ? undefined : valueSpecValue, xAxisVariable: vizConfig.xAxisVariable, yAxisVariable: vizConfig.yAxisVariable, overlayVariable: vizConfig.overlayVariable, @@ -724,12 +749,12 @@ function ScatterplotViz(props: VisualizationProps) { showMissingness: vizConfig.showMissingness ? 'TRUE' : 'FALSE', }, computeConfig: copmutationAppOverview.computeName - ? computation.descriptor.configuration + ? computationDescriptor.configuration : undefined, }; const response = await dataClient.getVisualizationData( - computation.descriptor.type, + computationDescriptor.type, visualization.descriptor.type, params, ScatterplotResponse @@ -741,7 +766,7 @@ function ScatterplotViz(props: VisualizationProps) { overlayEntity, overlayVariable, outputEntity, - filteredCounts.value, + filteredCounts, response.completeCasesTable ); const showMissingFacet = @@ -750,7 +775,7 @@ function ScatterplotViz(props: VisualizationProps) { facetEntity, facetVariable, outputEntity, - filteredCounts.value, + filteredCounts, response.completeCasesTable ); @@ -789,8 +814,7 @@ function ScatterplotViz(props: VisualizationProps) { showMissingFacet, facetVocabulary, facetVariable, - // pass computation - computation.descriptor.type, + computationDescriptor.type, entities, colorPaletteOverride ); @@ -798,44 +822,9 @@ function ScatterplotViz(props: VisualizationProps) { ...returnData, overlayValueToColorMapper, }; - }, [ - computedYAxisDetails, - computeJobStatus, - outputEntity, - filteredCounts.pending, - filteredCounts.value, - xAxisVariable, - yAxisVariable, - overlayVariable, - facetVariable, - inputsForValidation, - selectedVariables, - entities, - dataElementConstraints, - dataElementDependencyOrder, - filters, - vizConfig.valueSpecConfig, - vizConfig.xAxisVariable, - vizConfig.yAxisVariable, - vizConfig.overlayVariable, - vizConfig.facetVariable, - vizConfig.showMissingness, - computedXAxisDetails, - showLogScaleBanner, - showContinousOverlayBanner, - studyId, - options?.hideTrendlines, - computation.descriptor.configuration, - computation.descriptor.type, - dataClient, - visualization.descriptor.type, - overlayEntity, - facetEntity, - computedOverlayVariableDescriptor, - neutralPaletteProps.colorPalette, - overlayMin, - overlayMax, - ]) + }, + [dataRequestDeps], + 60 * 1000 // 60 seconds cache time ); const outputSize = From 65004b37ee7936ec7895b7eb4b19b40d86405032 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Mon, 18 Nov 2024 12:50:17 +0000 Subject: [PATCH 24/26] fix dependency warnings for histogram --- .../implementations/HistogramVisualization.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 3a5941e306..1db860585d 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 @@ -894,17 +894,18 @@ function HistogramViz(props: VisualizationProps) { ? { dependentAxisRange: minPosMax } : {}), }, - vizConfig, + { + independentAxisRange: vizConfig.independentAxisRange, + dependentAxisRange: vizConfig.dependentAxisRange, + }, {}, // no overrides true // use inclusive less than equal for the range min ), [ defaultUIState, - dependentMinPosMax, + minPosMax, vizConfig.independentAxisRange, vizConfig.dependentAxisRange, - vizConfig.independentAxisValueSpec, - vizConfig.dependentAxisValueSpec, ] ); From c29fde8b07d27e7f2ae16545a8145a2095af3675 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Tue, 19 Nov 2024 10:52:19 +0000 Subject: [PATCH 25/26] fix broken dependencies --- .../implementations/LineplotVisualization.tsx | 11 ++++++++--- packages/libs/eda/src/lib/core/hooks/cachedPromise.ts | 5 +++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx index af90bd616a..8329fdd6d1 100755 --- a/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx +++ b/packages/libs/eda/src/lib/core/components/visualizations/implementations/LineplotVisualization.tsx @@ -71,7 +71,6 @@ import { CoverageStatistics } from '../../../types/visualization'; // import axis label unit util import { variableDisplayWithUnit } from '../../../utils/variable-display'; import { - NumberVariable, DateVariable, StudyEntity, Variable, @@ -477,8 +476,10 @@ function LineplotViz(props: VisualizationProps) { vizConfig.dependentAxisLogScale, vizConfig.independentAxisValueSpec, vizConfig.dependentAxisValueSpec, + vizConfig.useBinning, findEntityAndVariable, updateVizConfig, + showMarginalHistogram, ] ); @@ -1183,7 +1184,11 @@ function LineplotViz(props: VisualizationProps) { }); // add reset for truncation message: including dependent axis warning as well setTruncatedIndependentAxisWarning(''); - }, [updateVizConfig, setTruncatedIndependentAxisWarning]); + }, [ + updateVizConfig, + setTruncatedIndependentAxisWarning, + alwaysEnableUseBinning, + ]); const handleDependentAxisRangeChange = useCallback( (newRange?: NumberOrDateRange) => { @@ -1214,7 +1219,7 @@ function LineplotViz(props: VisualizationProps) { }); // add reset for truncation message as well setTruncatedDependentAxisWarning(''); - }, [updateVizConfig, categoricalMode]); + }, [updateVizConfig, setTruncatedDependentAxisWarning]); // set useEffect for changing truncation warning message useEffect(() => { diff --git a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts index 6d387d7e65..80633f3d6c 100644 --- a/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts +++ b/packages/libs/eda/src/lib/core/hooks/cachedPromise.ts @@ -33,13 +33,14 @@ export function useCachedPromise( // Mapping the state from useQuery to PromiseHookState // and return something stable + const isPending = isLoading || isFetching; const state: PromiseHookState = useMemo( () => ({ value: enabled ? data : undefined, - pending: enabled && (isLoading || isFetching), + pending: enabled && isPending, error: error, }), - [data, enabled, isLoading || isFetching, error] + [data, enabled, isPending, error] ); return state; From deff6525528b047450eda9ed08047b6fc162a9b4 Mon Sep 17 00:00:00 2001 From: Bob MacCallum Date: Tue, 19 Nov 2024 12:04:45 +0000 Subject: [PATCH 26/26] fix multifilter when there is no filter applied to the variable --- .../core/components/filter/MultiFilter.tsx | 145 +++++++++--------- 1 file changed, 71 insertions(+), 74 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx b/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx index 64e2192ce3..71d42dde44 100644 --- a/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx +++ b/packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx @@ -193,81 +193,78 @@ export function MultiFilter(props: Props) { }, [_thisFilter, thisFilter]); // Counts retrieved from the backend, used for the table display. - const leafSummariesPromise = useCachedPromise( - () => { - return Promise.all( - leaves.map((leaf) => { - const thisFilterWithoutLeaf = thisFilter && { - ...thisFilter, - subFilters: thisFilter.subFilters.filter( - (f) => f.variableId !== leaf.term - ), - }; - return getDistribution( - { - entityId: entity.id, - variableId: leaf.term, - filters: - thisFilterWithoutLeaf == null || - thisFilterWithoutLeaf.subFilters.length === 0 || - thisFilterWithoutLeaf.operation === 'union' - ? otherFilters - : [...(otherFilters || []), thisFilterWithoutLeaf], - }, - (filters) => - subsettingClient.getDistribution( - studyMetadata.id, - entity.id, - leaf.term, - { - filters, - valueSpec: 'count', - } - ) - ).then((distribution) => { - const fgValueByLabel = Object.fromEntries( - distribution.foreground.histogram.map(({ binLabel, value }) => [ - binLabel, - value ?? 0, - ]) - ); - const bgValueByLabel = Object.fromEntries( - distribution.background.histogram.map(({ binLabel, value }) => [ - binLabel, - value ?? 0, - ]) + const leafSummariesPromise = useCachedPromise(() => { + return Promise.all( + leaves.map((leaf) => { + const thisFilterWithoutLeaf = thisFilter && { + ...thisFilter, + subFilters: thisFilter.subFilters.filter( + (f) => f.variableId !== leaf.term + ), + }; + return getDistribution( + { + entityId: entity.id, + variableId: leaf.term, + filters: + thisFilterWithoutLeaf == null || + thisFilterWithoutLeaf.subFilters.length === 0 || + thisFilterWithoutLeaf.operation === 'union' + ? otherFilters + : [...(otherFilters || []), thisFilterWithoutLeaf], + }, + (filters) => + subsettingClient.getDistribution( + studyMetadata.id, + entity.id, + leaf.term, + { + filters, + valueSpec: 'count', + } + ) + ).then((distribution) => { + const fgValueByLabel = Object.fromEntries( + distribution.foreground.histogram.map(({ binLabel, value }) => [ + binLabel, + value ?? 0, + ]) + ); + const bgValueByLabel = Object.fromEntries( + distribution.background.histogram.map(({ binLabel, value }) => [ + binLabel, + value ?? 0, + ]) + ); + const variable = variablesById[leaf.term]; + if (variable == null || !isTableVariable(variable)) + throw new Error( + `Could not find a categorical EDA variable associated with the leaf field "${leaf.term}".` ); - const variable = variablesById[leaf.term]; - if (variable == null || !isTableVariable(variable)) - throw new Error( - `Could not find a categorical EDA variable associated with the leaf field "${leaf.term}".` - ); - return { - term: leaf.term, - display: leaf.display, - valueCounts: variable.vocabulary?.map((label) => ({ - value: label, - count: bgValueByLabel[label], - filteredCount: fgValueByLabel[label] ?? 0, - })), - internalsCount: - distribution.background.statistics.numDistinctEntityRecords, - internalsFilteredCount: - distribution.foreground.statistics.numDistinctEntityRecords, - }; - }); - }) - ); - }, - [ - thisFilter, - otherFilters, - leaves, - entity.id, - studyMetadata.id, - variablesById, - ] // used to have `subsettingClient` - ); + return { + term: leaf.term, + display: leaf.display, + valueCounts: variable.vocabulary?.map((label) => ({ + value: label, + count: bgValueByLabel[label], + filteredCount: fgValueByLabel[label] ?? 0, + })), + internalsCount: + distribution.background.statistics.numDistinctEntityRecords, + internalsFilteredCount: + distribution.foreground.statistics.numDistinctEntityRecords, + }; + }); + }) + ); + }, [ + thisFilter ? thisFilter : { type: 'NO_FILTER' }, + otherFilters, + leaves, + entity.id, + studyMetadata.id, + variablesById, + ]); // Sorted counts. This is done separately from retrieving the data so that // updates to sorting don't incur backend requests.