Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use useQuery in place of usePromise for EDA subsetting and visualizations #1228

Merged
merged 30 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1619749
WIP but not working
bobular Oct 11, 2024
6a7f6d0
react-query working now for subsetting
bobular Oct 11, 2024
95aaead
basic approach working but still WIP
bobular Oct 17, 2024
226d775
missing dep
bobular Oct 17, 2024
b3c0f59
another missing dep
bobular Oct 17, 2024
72bac14
Merge remote-tracking branch 'origin/main' into 1103-react-query-retr…
bobular Oct 17, 2024
db8aa66
improve comments
bobular Oct 17, 2024
d5a2c6d
solve userdb PATCH update issue with referential stability
bobular Oct 17, 2024
deca1d9
spinner behaviour hopefully fixed
bobular Oct 17, 2024
cd3e13e
cancel variable works and old data is flushed when appropriate
bobular Oct 17, 2024
1a3ede4
add cacheTime argument to useCachedPromise for the future
bobular Oct 18, 2024
4484b0c
map supporting plots now behave correctly
bobular Oct 18, 2024
a6afabf
rename to dataRequestDeps and document it
bobular Oct 18, 2024
5fd09c8
done barplot
bobular Oct 18, 2024
b782daf
remove commented deps
bobular Oct 18, 2024
a098e43
standardise throw messages
bobular Oct 18, 2024
12d43bd
boxplot
bobular Oct 18, 2024
db32ab4
fix mbio boxplot behaviour
bobular Oct 19, 2024
541b59a
started with lineplot
bobular Oct 21, 2024
b840c6e
fix oopsie with unique vars check
bobular Oct 21, 2024
0e60f89
fixed lineplot incompatible settings banner behaviour and used variab…
bobular Oct 21, 2024
03d39ec
take care of floating plot overlay and remove commented deps
bobular Oct 21, 2024
bf3c641
Merge remote-tracking branch 'origin/main' into 1103-react-query-retr…
bobular Oct 24, 2024
9994ca2
mosaics done
bobular Oct 25, 2024
f66f3c1
scatterplot
bobular Oct 26, 2024
1267fa7
Merge branch 'main' into 1103-react-query-retrofit
bobular Nov 8, 2024
65004b3
fix dependency warnings for histogram
bobular Nov 18, 2024
c29fde8
fix broken dependencies
bobular Nov 19, 2024
deff652
fix multifilter when there is no filter applied to the variable
bobular Nov 19, 2024
0415b50
Merge remote-tracking branch 'origin/main' into 1103-react-query-retr…
bobular Nov 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/libs/eda/src/lib/core/api/queryClient.ts
Original file line number Diff line number Diff line change
@@ -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,
},
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -220,17 +218,14 @@ export function HistogramFilter(props: Props) {
};
},
[
dataParams,
otherFilters,
entity.displayName,
entity.displayNamePlural,
entity.id,
studyMetadata.id,
subsettingClient,
variable,
]
);
const data = usePromise(
useCallback(() => getData(uiStateForData), [getData, uiStateForData])
] // used to have `subsettingClient`
);

const filter = filters?.find(
Expand Down
147 changes: 72 additions & 75 deletions packages/libs/eda/src/lib/core/components/filter/MultiFilter.tsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All "MultiFilter" variables appear to be broken. An example can be found with a clinepi-site (using qa.clinepi): http://localhost:8080/a/app/workspace/analyses/DS_841a9f5259/new/variables/PCO_0000024/ENVO_00003064.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because one of the deps is nullish:

const enabled = queryKey.every((val) => val != null);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dmfalke - I have fixed this now.
I did test it but I must have already checked some checkboxes before deploying the useQuery.

Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -193,81 +193,78 @@ export function MultiFilter(props: Props) {
}, [_thisFilter, thisFilter]);

// Counts retrieved from the backend, used for the table display.
const leafSummariesPromise = usePromise(
useCallback(() => {
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,
subsettingClient,
studyMetadata.id,
variablesById,
])
);
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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<DistributionResponse>(
{
entityId: entity.id,
Expand Down Expand Up @@ -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(
() => ({
Expand Down
Loading
Loading