Skip to content

More sophisticated showMissing behaviour and reused more entity counts #770

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

Merged
merged 7 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions src/lib/core/components/VariableCoverageTable.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { CompleteCasesTable } from '../api/DataClient';
import { EntityCounts } from '../hooks/entityCounts';
import { PromiseHookState } from '../hooks/promise';
import { useVariableCoverageTableRows } from '../hooks/variableCoverage';
import { Filter } from '../types/filter';
import { VariableDescriptor } from '../types/variable';

export interface Props {
containerClassName?: string;
completeCases?: CompleteCasesTable;
filters?: Filter[];
filteredCounts: PromiseHookState<EntityCounts>;
variableSpecs: VariableSpec[];
outputEntityId?: string;
}
Expand Down Expand Up @@ -42,13 +43,13 @@ export interface VariableCoverageTableRow {
export function VariableCoverageTable({
containerClassName,
completeCases,
filters,
filteredCounts,
outputEntityId,
variableSpecs,
}: Props) {
const rows = useVariableCoverageTableRows(
variableSpecs,
filters,
filteredCounts,
completeCases,
outputEntityId
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ import { scatterplotVisualization } from '../visualizations/implementations/Scat
import { barplotVisualization } from '../visualizations/implementations/BarplotVisualization';
import { boxplotVisualization } from '../visualizations/implementations/BoxplotVisualization';
import { EntityCounts } from '../../hooks/entityCounts';
import { PromiseHookState } from '../../hooks/promise';

interface Props {
analysisState: AnalysisState;
computationAppOverview: ComputationAppOverview;
totalCounts: EntityCounts | undefined;
filteredCounts: EntityCounts | undefined;
totalCounts: PromiseHookState<EntityCounts>;
filteredCounts: PromiseHookState<EntityCounts>;
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/lib/core/components/visualizations/VisualizationTypes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { EntityCounts } from '../../hooks/entityCounts';
import { PromiseHookState } from '../../hooks/promise';
import { Filter } from '../../types/filter';
import { VariableDescriptor } from '../../types/variable';
import {
Expand All @@ -21,8 +22,8 @@ export interface VisualizationProps {
filters?: Filter[];
starredVariables: VariableDescriptor[];
toggleStarredVariable: (targetVariableId: VariableDescriptor) => void;
totalCounts: EntityCounts | undefined;
filteredCounts: EntityCounts | undefined;
totalCounts: PromiseHookState<EntityCounts>;
filteredCounts: PromiseHookState<EntityCounts>;
}

export type SelectorProps = VisualizationOverview;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import PlaceholderIcon from './PlaceholderIcon';
import { Tooltip } from '@material-ui/core';
import { isEqual } from 'lodash';
import { EntityCounts } from '../../hooks/entityCounts';
import { PromiseHookState } from '../../hooks/promise';

const cx = makeClassNameHelper('VisualizationsContainer');

Expand All @@ -42,8 +43,8 @@ interface Props {
filters: Filter[];
starredVariables: VariableDescriptor[];
toggleStarredVariable: (targetVariable: VariableDescriptor) => void;
totalCounts: EntityCounts | undefined;
filteredCounts: EntityCounts | undefined;
totalCounts: PromiseHookState<EntityCounts>;
filteredCounts: PromiseHookState<EntityCounts>;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ import {
fixLabelForNumberVariables,
fixLabelsForNumberVariables,
grayOutLastSeries,
omitEmptyNoDataSeries,
vocabularyWithMissingData,
variablesAreUnique,
nonUniqueWarning,
hasIncompleteCases,
} from '../../../utils/visualization';
import { VariablesByInputName } from '../../../utils/data-element-constraints';
// use lodash instead of Math.min/max
Expand Down Expand Up @@ -208,15 +208,24 @@ function BarplotViz(props: VisualizationProps) {
);

const findEntityAndVariable = useFindEntityAndVariable(entities);
const { variable, entity, overlayVariable, facetVariable } = useMemo(() => {
const {
variable,
entity,
overlayVariable,
overlayEntity,
facetVariable,
facetEntity,
} = useMemo(() => {
const xAxisVariable = findEntityAndVariable(vizConfig.xAxisVariable);
const overlayVariable = findEntityAndVariable(vizConfig.overlayVariable);
const facetVariable = findEntityAndVariable(vizConfig.facetVariable);
return {
variable: xAxisVariable?.variable,
entity: xAxisVariable?.entity,
overlayVariable: overlayVariable?.variable,
overlayEntity: overlayVariable?.entity,
facetVariable: facetVariable?.variable,
facetEntity: facetVariable?.entity,
};
}, [
findEntityAndVariable,
Expand All @@ -227,23 +236,45 @@ function BarplotViz(props: VisualizationProps) {

const data = usePromise(
useCallback(async (): Promise<BarplotDataWithStatistics | undefined> => {
if (variable == null) return undefined;
if (
variable == null ||
entity == null ||
filteredCounts.pending ||
filteredCounts.value == null
)
return undefined;

if (!variablesAreUnique([variable, overlayVariable, facetVariable]))
throw new Error(nonUniqueWarning);

const params = getRequestParams(studyId, filters ?? [], vizConfig);

const response = dataClient.getBarplot(
const response = await dataClient.getBarplot(
computation.descriptor.type,
params as BarplotRequestParams
);

const showMissing =
vizConfig.showMissingness &&
(overlayVariable != null || facetVariable != null);
// 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 =
vizConfig.showMissingness && overlayVariable != null;
vizConfig.showMissingness &&
hasIncompleteCases(
overlayEntity,
overlayVariable,
entity,
filteredCounts.value,
response.completeCasesTable
);
const showMissingFacet =
vizConfig.showMissingness &&
hasIncompleteCases(
facetEntity,
facetVariable,
entity,
filteredCounts.value,
response.completeCasesTable
);

const vocabulary = fixLabelsForNumberVariables(
variable?.vocabulary,
variable
Expand All @@ -257,34 +288,33 @@ function BarplotViz(props: VisualizationProps) {
facetVariable
);

return omitEmptyNoDataSeries(
grayOutLastSeries(
reorderData(
barplotResponseToData(
await response,
variable,
overlayVariable,
facetVariable
),
vocabulary,
vocabularyWithMissingData(overlayVocabulary, showMissing),
vocabularyWithMissingData(facetVocabulary, showMissing)
return grayOutLastSeries(
reorderData(
barplotResponseToData(
response,
variable,
overlayVariable,
facetVariable
),
showMissingOverlay
vocabulary,
vocabularyWithMissingData(overlayVocabulary, showMissingOverlay),
vocabularyWithMissingData(facetVocabulary, showMissingFacet)
),
showMissing
showMissingOverlay
);
}, [
// using vizConfig only causes issue with onCheckedLegendItemsChange
studyId,
filters,
filteredCounts,
dataClient,
vizConfig.xAxisVariable,
vizConfig.overlayVariable,
vizConfig.facetVariable,
vizConfig.valueSpec,
vizConfig.showMissingness,
variable,
entity,
overlayVariable,
computation.descriptor.type,
])
Expand Down Expand Up @@ -401,7 +431,7 @@ function BarplotViz(props: VisualizationProps) {
vizConfig.valueSpec === 'count' ? 'Count' : 'Proportion',
legendTitle: overlayVariable?.displayName,
interactive: true,
showSpinner: data.pending,
showSpinner: data.pending || filteredCounts.pending,
dependentAxisLogScale: vizConfig.dependentAxisLogScale,
// set dependent axis range for log scale
dependentAxisRange: dependentAxisRange,
Expand Down Expand Up @@ -473,12 +503,12 @@ function BarplotViz(props: VisualizationProps) {
outputEntity={entity}
stratificationIsActive={overlayVariable != null}
enableSpinner={vizConfig.xAxisVariable != null && !data.error}
totalCounts={totalCounts}
filteredCounts={filteredCounts}
totalCounts={totalCounts.value}
filteredCounts={filteredCounts.value}
/>
<VariableCoverageTable
completeCases={data.pending ? undefined : data.value?.completeCases}
filters={filters}
filteredCounts={filteredCounts}
outputEntityId={vizConfig.xAxisVariable?.entityId}
variableSpecs={[
{
Expand Down
Loading