From bdd69160e20911d7988cb30ca6dbdd4a622a259c Mon Sep 17 00:00:00 2001 From: Matt Weiden <538456+mweiden@users.noreply.github.com> Date: Thu, 5 Mar 2020 18:04:14 -0800 Subject: [PATCH] Do not over-prune var data cache (#1198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove unused variables and imports * Simplify conditional * Fix typo * Do not overprune var data cache There is a bug in how the universe and world gene sets are constructed and passed to `ControlsHelpers.pruneVarDataCache` that causes the var data cache to be over-pruned. This commit fixes the issue. Consider the following example from the node console: ``` ❯ node Welcome to Node.js v13.5.0. Type ".help" for more information. > new Set([1], [2], [3]) Set(1) { 1 } ``` What we really want is the set `Set(3) { 1, 2, 3 }`, which can be constructed as: ``` > new Set([].concat([1], [2], [3])) Set(3) { 1, 2, 3 } ``` --- client/src/actions/index.js | 6 +++--- client/src/components/categorical/category.js | 2 +- client/src/components/graph/graph.js | 3 +-- client/src/reducers/controls.js | 1 - client/src/reducers/undoable.js | 6 +++--- client/src/reducers/undoableConfig.js | 2 +- client/src/reducers/universe.js | 4 +--- client/src/reducers/world.js | 9 ++------- client/src/util/stateManager/controlsHelpers.js | 2 +- 9 files changed, 13 insertions(+), 22 deletions(-) diff --git a/client/src/actions/index.js b/client/src/actions/index.js index fb640e5f..3cab18d8 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -127,14 +127,14 @@ const doInitialDataLoad = () => Step 2 - load the minimum stuff required to display. */ await Promise.all([ - layoutFetchAndLoad(dispatch, schema, universe), - varAnnotationFetchAndLoad(dispatch, schema, universe) + layoutFetchAndLoad(dispatch), + varAnnotationFetchAndLoad(dispatch, schema) ]); /* Step 3 - load everything else */ - await obsAnnotationFetchAndLoad(dispatch, schema, universe); + await obsAnnotationFetchAndLoad(dispatch, schema); dispatch({ type: "initial data load complete (universe exists)", diff --git a/client/src/components/categorical/category.js b/client/src/components/categorical/category.js index 67952354..bd461f2f 100644 --- a/client/src/components/categorical/category.js +++ b/client/src/components/categorical/category.js @@ -90,7 +90,7 @@ class Category extends React.Component { // || this.checkbox.indeterminate === false if (isChecked) { this.toggleNone(); - } else if (!isChecked) { + } else { this.toggleAll(); } } diff --git a/client/src/components/graph/graph.js b/client/src/components/graph/graph.js index 478d9b41..aa8f19a6 100644 --- a/client/src/components/graph/graph.js +++ b/client/src/components/graph/graph.js @@ -422,8 +422,7 @@ class Graph extends React.Component { handleEnd, handleCancel, responsive, - this.graphPaddingRightLeft, - graphInteractionMode + this.graphPaddingRightLeft ); return { toolSVG: newToolSVG, tool, container }; diff --git a/client/src/reducers/controls.js b/client/src/reducers/controls.js index d38b3dac..3bb4d459 100644 --- a/client/src/reducers/controls.js +++ b/client/src/reducers/controls.js @@ -1,7 +1,6 @@ // jshint esversion: 6 import _ from "lodash"; -import * as globals from "../globals"; import { subsetAndResetGeneLists } from "../util/stateManager/controlsHelpers"; const Controls = ( diff --git a/client/src/reducers/undoable.js b/client/src/reducers/undoable.js index 3cf9f42c..015cfff1 100644 --- a/client/src/reducers/undoable.js +++ b/client/src/reducers/undoable.js @@ -221,15 +221,15 @@ const Undoable = (reducer, undoableKeys, options = {}) => { const aType = action.type; switch (aType) { case "@@undoable/undo": { - return undo(currentState, action); + return undo(currentState); } case "@@undoable/redo": { - return redo(currentState, action); + return redo(currentState); } case "@@undoable/clear": { - return clear(currentState, action); + return clear(currentState); } default: { diff --git a/client/src/reducers/undoableConfig.js b/client/src/reducers/undoableConfig.js index 2d08f4ca..8c79591b 100644 --- a/client/src/reducers/undoableConfig.js +++ b/client/src/reducers/undoableConfig.js @@ -145,7 +145,7 @@ Signature: (fsm, event, from) => undoableAction const onFsmError = (fsm, event, from) => { console.error(`FSM error [event: "${event}", state: "${from}"]`, fsm); // In production, try to recover gracefully if we have unexpected state - return clear(fsm); + return clear(); }; /* diff --git a/client/src/reducers/universe.js b/client/src/reducers/universe.js index 54e45aaa..bdb99390 100644 --- a/client/src/reducers/universe.js +++ b/client/src/reducers/universe.js @@ -62,9 +62,7 @@ const Universe = (state = null, action, nextSharedState, prevSharedState) => { const { userDefinedGenes, diffexpGenes } = prevSharedState; const allTheGenesWeNeed = [ ...new Set( - userDefinedGenes, - diffexpGenes, - Object.keys(action.expressionData) + [userDefinedGenes, diffexpGenes, Object.keys(action.expressionData)].filter(ele => ele).flat() ) ]; varData = ControlsHelpers.pruneVarDataCache(varData, allTheGenesWeNeed); diff --git a/client/src/reducers/world.js b/client/src/reducers/world.js index 61a83ecf..ee93a363 100644 --- a/client/src/reducers/world.js +++ b/client/src/reducers/world.js @@ -101,15 +101,10 @@ const WorldReducer = ( const { userDefinedGenes, diffexpGenes } = prevSharedState; const allTheGenesWeNeed = [ ...new Set( - userDefinedGenes, - diffexpGenes, - Object.keys(action.expressionData) + [userDefinedGenes, diffexpGenes, Object.keys(action.expressionData)].filter(ele => ele).flat() ) ]; - unclippedVarData = ControlsHelpers.pruneVarDataCache( - unclippedVarData, - allTheGenesWeNeed - ); + unclippedVarData = ControlsHelpers.pruneVarDataCache(unclippedVarData, allTheGenesWeNeed); // at this point, we have the unclipped data in unclippedVarData. // Now create clipped. diff --git a/client/src/util/stateManager/controlsHelpers.js b/client/src/util/stateManager/controlsHelpers.js index ed51a652..5d0f94ea 100644 --- a/client/src/util/stateManager/controlsHelpers.js +++ b/client/src/util/stateManager/controlsHelpers.js @@ -172,7 +172,7 @@ export function pruneVarDataCache(varData, needed) { */ /* - VarDataCacheLowWatermark - this cofig value sets the minimum cache size, + VarDataCacheLowWatermark - this config value sets the minimum cache size, in columns, below which we don't throw away data. The value should be high enough so we are caching the maximum which will