Merge pull request #770 from VEuPathDB/issue-676

More sophisticated showMissing behaviour and reused more entity counts

Author: bobular

src/lib/core/components/VariableCoverageTable.tsx

@@ -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;
 }
@@ -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
   );

src/lib/core/components/computations/PassThroughComputation.tsx

@@ -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>;
 }
 
 /**

src/lib/core/components/visualizations/VisualizationTypes.ts

@@ -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 {
@@ -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;

src/lib/core/components/visualizations/VisualizationsContainer.tsx

@@ -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');
 
@@ -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>;
 }
 
 /**

src/lib/core/components/visualizations/implementations/BarplotVisualization.tsx

@@ -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
@@ -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,
@@ -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
@@ -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,
     ])
@@ -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,
@@ -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={[
           {