From 2b77f8146bacb4010172d8ccf68b4b06875a92d2 Mon Sep 17 00:00:00 2001
From: Colin Megill <colinmegill@gmail.com>
Date: Thu, 11 Oct 2018 16:13:24 -0700
Subject: [PATCH] namespacing

---
 .../components/brushableHistogram/index.js    | 13 ++-
 .../src/components/continuous/continuous.js   |  1 +
 client/src/components/geneExpression/index.js | 30 +++---
 client/src/reducers/controls.js               | 99 ++++++++++++-------
 client/src/reducers/differential.js           |  3 +-
 client/src/util/nameCreators.js               | 43 ++++++++
 client/src/util/stateManager/world.js         | 19 ++--
 7 files changed, 151 insertions(+), 57 deletions(-)
 create mode 100644 client/src/util/nameCreators.js

diff --git a/client/src/components/brushableHistogram/index.js b/client/src/components/brushableHistogram/index.js
index 72642b54..b1b36527 100644
--- a/client/src/components/brushableHistogram/index.js
+++ b/client/src/components/brushableHistogram/index.js
@@ -83,17 +83,28 @@ class HistogramBrush extends React.Component {
 
   onBrush(selection, x) {
     return () => {
-      const { dispatch, field } = this.props;
+      const { dispatch, field, isObs, isUserDefined, isDiffExp } = this.props;
+
       if (d3.event.selection) {
         dispatch({
           type: "continuous metadata histogram brush",
           selection: field,
+          continuousNamespace: {
+            isObs,
+            isUserDefined,
+            isDiffExp
+          },
           range: [x(d3.event.selection[0]), x(d3.event.selection[1])]
         });
       } else {
         dispatch({
           type: "continuous metadata histogram brush",
           selection: field,
+          continuousNamespace: {
+            isObs,
+            isUserDefined,
+            isDiffExp
+          },
           range: null
         });
       }
diff --git a/client/src/components/continuous/continuous.js b/client/src/components/continuous/continuous.js
index 78976616..74242411 100644
--- a/client/src/components/continuous/continuous.js
+++ b/client/src/components/continuous/continuous.js
@@ -58,6 +58,7 @@ class Continuous extends React.Component {
               <HistogramBrush
                 key={key}
                 field={key}
+                isObs
                 fieldValues={obsAnnotations}
                 ranges={value.range}
                 handleColorAction={this.handleColorAction(key).bind(this)}
diff --git a/client/src/components/geneExpression/index.js b/client/src/components/geneExpression/index.js
index 77abc929..864c5f2a 100644
--- a/client/src/components/geneExpression/index.js
+++ b/client/src/components/geneExpression/index.js
@@ -5,11 +5,11 @@ import React from "react";
 import _ from "lodash";
 import * as d3 from "d3";
 import { connect } from "react-redux";
-import HistogramBrush from "../brushableHistogram";
 import CirclePlus from "react-icons/lib/fa/plus-circle";
+import HistogramBrush from "../brushableHistogram";
 import * as globals from "../../globals";
 // import ReactAutocomplete from "react-autocomplete"; /* http://emilebres.github.io/react-virtualized-checkbox/ */
-import actions from "../../actions/";
+import actions from "../../actions";
 
 @connect(state => {
   const metadata = _.get(state.controls.world, "obsAnnotations", null);
@@ -22,7 +22,6 @@ import actions from "../../actions/";
     initializeRanges,
     userDefinedGenes: state.controls.userDefinedGenes,
     world: state.controls.world,
-    dimensionMap: state.controls.dimensionMap,
     colorAccessor: state.controls.colorAccessor,
     allGeneNames: state.controls.allGeneNames,
     differential: state.differential
@@ -32,40 +31,42 @@ class GeneExpression extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      svg: null,
-      ctx: null,
-      axes: null,
-      dimensions: null,
       gene: ""
     };
   }
+
   keyPress(e) {
-    if (e.keyCode == 13) {
+    if (e.keyCode === 13) {
       this.handleClick();
     }
   }
+
   handleClick() {
     const { world, dispatch, userDefinedGenes } = this.props;
+    const { gene } = this.state;
 
-    if (userDefinedGenes.indexOf(this.state.gene) !== -1) {
+    if (userDefinedGenes.indexOf(gene) !== -1) {
       console.log("That gene already exists");
     } else if (userDefinedGenes.length > 15) {
       console.log(
         "That's too many genes, you can have at most 15 user defined genes"
       );
-    } else if (!_.find(world.varAnnotations, { name: this.state.gene })) {
+    } else if (!_.find(world.varAnnotations, { name: gene })) {
       console.log("That doesn't appear to be a valid gene name.");
     } else {
-      dispatch(actions.requestGeneExpressionCountsPOST([this.state.gene]));
+      dispatch(actions.requestGeneExpressionCountsPOST([gene]));
       dispatch({
         type: "user defined gene",
-        data: this.state.gene
+        data: gene
       });
       this.setState({ gene: "" });
     }
   }
+
   render() {
     const { world, userDefinedGenes } = this.props;
+    const { gene } = this.state;
+
     return (
       <div>
         <input
@@ -74,9 +75,10 @@ class GeneExpression extends React.Component {
             this.setState({ gene: e.target.value });
           }}
           type="text"
-          value={this.state.gene}
+          value={gene}
         />
         <button
+          type="button"
           style={{
             border: "none",
             background: "none",
@@ -101,7 +103,7 @@ class GeneExpression extends React.Component {
           <p>User defined genes</p>
         ) : null}
         {world && userDefinedGenes.length > 0
-          ? _.map(userDefinedGenes, (geneName, index) => {
+          ? _.map(userDefinedGenes, geneName => {
               if (!world.varDataCache[geneName]) {
                 return null;
               } else {
diff --git a/client/src/reducers/controls.js b/client/src/reducers/controls.js
index d1bacee9..ee512fb1 100644
--- a/client/src/reducers/controls.js
+++ b/client/src/reducers/controls.js
@@ -5,6 +5,13 @@ import { World, kvCache } from "../util/stateManager";
 import parseRGB from "../util/parseRGB";
 import Crossfilter from "../util/typedCrossfilter";
 import * as globals from "../globals";
+import {
+  layoutDimensionName,
+  obsAnnoDimensionName,
+  userDefinedDimensionName,
+  diffexpDimensionName,
+  makeContinuousDimensionName
+} from "../util/nameCreators";
 
 function createCategoricalAsBooleansMap(world) {
   const res = {};
@@ -79,8 +86,14 @@ const Controls = (
       const crossfilter = Crossfilter(world.obsAnnotations);
       const dimensionMap = World.createObsDimensionMap(crossfilter, world);
 
-      let worldVarDataCache = world.varDataCache;
+      const worldVarDataCache = world.varDataCache;
 
+      // dimensionMap = {
+      //     layout_X: dim-for-X,
+      //     obsAnno_name: dim for an annotation,
+      //     varData_userDefined_genename: dim for user defined expression,
+      //     varData_diffexp_genename: dim for diff-exp added gene expression
+      // }
       /* var dimensions */
       if (userDefinedGenes.length > 0) {
         /*
@@ -88,8 +101,10 @@ const Controls = (
           in controls rather than an array, should be abstracted into
           util ie., createDimensionsFromBothListsOfGenes(userGenes, diffExp)
         */
-        _.forEach(userDefinedGenes, (gene, index) => {
-          dimensionMap[gene] = World.createVarDimension(
+        _.forEach(userDefinedGenes, gene => {
+          dimensionMap[
+            userDefinedDimensionName(gene)
+          ] = World.createVarDimension(
             /* "__var__" + */
             world,
             worldVarDataCache,
@@ -100,8 +115,8 @@ const Controls = (
       }
 
       if (diffexpGenes.length > 0) {
-        _.forEach(diffexpGenes, (gene, index) => {
-          dimensionMap[gene] = World.createVarDimension(
+        _.forEach(diffexpGenes, gene => {
+          dimensionMap[diffexpDimensionName(gene)] = World.createVarDimension(
             /* "__var__" + */
             world,
             worldVarDataCache,
@@ -140,7 +155,7 @@ const Controls = (
       const crossfilter = Crossfilter(world.obsAnnotations);
       const dimensionMap = World.createObsDimensionMap(crossfilter, world);
 
-      let worldVarDataCache = world.varDataCache;
+      const worldVarDataCache = world.varDataCache;
       /* var dimensions */
 
       if (userDefinedGenes.length > 0) {
@@ -149,8 +164,10 @@ const Controls = (
           in controls rather than an array, should be abstracted into
           util ie., createDimensionsFromBothListsOfGenes(userGenes, diffExp)
         */
-        _.forEach(userDefinedGenes, (gene, index) => {
-          dimensionMap[gene] = World.createVarDimension(
+        _.forEach(userDefinedGenes, gene => {
+          dimensionMap[
+            userDefinedDimensionName(gene)
+          ] = World.createVarDimension(
             /* "__var__" + */
             world,
             worldVarDataCache,
@@ -161,8 +178,8 @@ const Controls = (
       }
 
       if (diffexpGenes.length > 0) {
-        _.forEach(diffexpGenes, (gene, index) => {
-          dimensionMap[gene] = World.createVarDimension(
+        _.forEach(diffexpGenes, gene => {
+          dimensionMap[diffexpDimensionName(gene)] = World.createVarDimension(
             /* "__var__" + */
             world,
             worldVarDataCache,
@@ -186,6 +203,7 @@ const Controls = (
       };
     }
     case "expression load success": {
+      console.log("expression load success", action);
       const { world, universe, crossfilter, dimensionMap } = state;
       let universeVarDataCache = universe.varDataCache;
       let worldVarDataCache = world.varDataCache;
@@ -223,33 +241,34 @@ const Controls = (
       };
     }
     case "request differential expression success": {
+      console.log("request diff success", action);
       const { world } = state;
       const _diffexpGenes = [];
+
       action.data.forEach(d => {
         _diffexpGenes.push(world.varAnnotations[d[0]].name);
       });
+
       return {
         ...state,
         diffexpGenes: _diffexpGenes
       };
     }
-    case "clear differential expression":
+    case "clear differential expression": {
       const { world, universe, crossfilter, dimensionMap } = state;
       const _dimensionMap = dimensionMap;
-      let universeVarDataCache = universe.varDataCache;
-      let worldVarDataCache = world.varDataCache;
+      const universeVarDataCache = universe.varDataCache;
+      const worldVarDataCache = world.varDataCache;
 
       _.forEach(action.diffExp, values => {
-        const name = world.varAnnotations[values[0]].name;
+        const { name } = world.varAnnotations[values[0]].name;
         // clean up crossfilter dimensions
-        const filterID = dimensionMap[name];
-        const dimension = crossfilter.filters.find(d => {
-          return (d.id = filterID);
-        });
+        const filterID = dimensionMap[diffexpDimensionName(name)];
+        const dimension = crossfilter.filters.find(d => d.id === filterID);
         dimension.dim.dispose();
 
         // clean up dimensionsMap
-        delete _dimensionMap[name];
+        delete _dimensionMap[diffexpDimensionName(name)];
         // clean up the varDataCaches
         delete universeVarDataCache[name];
         delete worldVarDataCache[name];
@@ -267,8 +286,12 @@ const Controls = (
           varDataCache: worldVarDataCache
         }
       };
+    }
     case "user defined gene": {
-      /* this could also live in expression success with a conditional, but that handles diffexp also */
+      /*
+        this could also live in expression success with a conditional,
+        but that handles diffexp also
+      */
       const newUserDefinedGenes = state.userDefinedGenes.slice();
       newUserDefinedGenes.push(action.data);
       return {
@@ -278,9 +301,10 @@ const Controls = (
     }
     case "clear user defined gene": {
       const { userDefinedGenes } = state;
-      const newUserDefinedGenes = _.filter(userDefinedGenes, d => {
-        return d !== action.data;
-      });
+      const newUserDefinedGenes = _.filter(
+        userDefinedGenes,
+        d => d !== action.data
+      );
       return {
         ...state,
         userDefinedGenes: newUserDefinedGenes
@@ -310,11 +334,11 @@ const Controls = (
              User Events
      *******************************/
     case "graph brush selection change": {
-      state.dimensionMap.x.filterRange([
+      state.dimensionMap[layoutDimensionName("X")].filterRange([
         action.brushCoords.northwest[0],
         action.brushCoords.southeast[0]
       ]);
-      state.dimensionMap.y.filterRange([
+      state.dimensionMap[layoutDimensionName("Y")].filterRange([
         action.brushCoords.southeast[1],
         action.brushCoords.northwest[1]
       ]);
@@ -324,20 +348,25 @@ const Controls = (
       };
     }
     case "graph brush deselect": {
-      state.dimensionMap.x.filterAll();
-      state.dimensionMap.y.filterAll();
+      state.dimensionMap[layoutDimensionName("X")].filterAll();
+      state.dimensionMap[layoutDimensionName("Y")].filterAll();
       return {
         ...state,
         graphBrushSelection: null
       };
     }
     case "continuous metadata histogram brush": {
+      const name = makeContinuousDimensionName(
+        action.continuousNamespace,
+        action.selection
+      );
+
       // action.selection: metadata name being selected
       // action.range: filter range, or null if deselected
       if (!action.range) {
-        state.dimensionMap[action.selection].filterAll();
+        state.dimensionMap[name].filterAll();
       } else {
-        state.dimensionMap[action.selection].filterRange(action.range);
+        state.dimensionMap[name].filterRange(action.range);
       }
       return { ...state };
     }
@@ -359,7 +388,7 @@ const Controls = (
         }
       };
       // update the filter for the one category that changed state
-      state.dimensionMap[action.metadataField].filterEnum(
+      state.dimensionMap[obsAnnoDimensionName(action.metadataField)].filterEnum(
         _.filter(
           _.map(
             newCategoricalAsBooleansMap[action.metadataField],
@@ -381,7 +410,7 @@ const Controls = (
         }
       };
       // update the filter for the one category that changed state
-      state.dimensionMap[action.metadataField].filterEnum(
+      state.dimensionMap[obsAnnoDimensionName(action.metadataField)].filterEnum(
         _.filter(
           _.map(
             newCategoricalAsBooleansMap[action.metadataField],
@@ -404,7 +433,9 @@ const Controls = (
           c[k] = false;
         }
       );
-      state.dimensionMap[action.metadataField].filterNone();
+      state.dimensionMap[
+        obsAnnoDimensionName(action.metadataField)
+      ].filterNone();
       return {
         ...state,
         categoricalAsBooleansMap: newCategoricalAsBooleansMap
@@ -420,7 +451,9 @@ const Controls = (
           c[k] = true;
         }
       );
-      state.dimensionMap[action.metadataField].filterAll();
+      state.dimensionMap[
+        obsAnnoDimensionName(action.metadataField)
+      ].filterAll();
       return {
         ...state,
         categoricalAsBooleansMap: newCategoricalAsBooleansMap
diff --git a/client/src/reducers/differential.js b/client/src/reducers/differential.js
index 9dacec70..6fa25926 100644
--- a/client/src/reducers/differential.js
+++ b/client/src/reducers/differential.js
@@ -1,5 +1,4 @@
 // jshint esversion: 6
-import _ from "lodash";
 
 const Differential = (
   state = {
@@ -47,7 +46,7 @@ const Differential = (
         diffExp: null,
         celllist1: null,
         celllist2: null
-      }
+      };
     case "reset World to eq Universe":
     case "set World to current selection":
       return {
diff --git a/client/src/util/nameCreators.js b/client/src/util/nameCreators.js
new file mode 100644
index 00000000..e7ec1612
--- /dev/null
+++ b/client/src/util/nameCreators.js
@@ -0,0 +1,43 @@
+/*
+
+The original motivation for this file is name collision. It's possible
+to have two of the same gene, user defined and diffexpressed, on the client
+simultaneously, looking at the same data in the cache but with two
+dimensions and two crossfilters. It's also possible that someone could
+have a obsAnnotation named X, but we are using that for layout. So
+we namespace, and abstract to avoid proliferating strings throughout the
+codebase.
+
+*/
+
+const makeDimensionName = (namespace, key) => `${namespace}_${key}`;
+
+export const layoutDimensionName = key => makeDimensionName("layout", key);
+export const obsAnnoDimensionName = key => makeDimensionName("obsAnno", key);
+export const diffexpDimensionName = key =>
+  makeDimensionName("varData_diffexp", key);
+export const userDefinedDimensionName = key =>
+  makeDimensionName("varData_userDefined", key);
+
+/*
+    continuousNamespace = {
+    isObs: true,
+    isDiffExp: false,
+    isUserDefined: false
+  }
+
+  ie., makeContinuousDimensionName(continuousNamespace = {isObs: true}, "total_reads")
+  see: histogram brush, as it doesn't know what type of continuous it was with only field
+*/
+export const makeContinuousDimensionName = (continuousNamespace, key) => {
+  let name;
+  if (continuousNamespace.isObs) {
+    name = obsAnnoDimensionName(key);
+  } else if (continuousNamespace.isDiffExp) {
+    name = diffexpDimensionName(key);
+  } else if (continuousNamespace.isUserDefined) {
+    name = userDefinedDimensionName(key);
+  }
+
+  return name;
+};
diff --git a/client/src/util/stateManager/world.js b/client/src/util/stateManager/world.js
index 87230cc1..50f7f8be 100644
--- a/client/src/util/stateManager/world.js
+++ b/client/src/util/stateManager/world.js
@@ -2,6 +2,7 @@
 
 import _ from "lodash";
 import * as kvCache from "./keyvalcache";
+import { layoutDimensionName, obsAnnoDimensionName } from "../nameCreators";
 
 /*
 World is a subset of universe.   Most code should use world, and should
@@ -296,13 +297,14 @@ export function createVarDimension(
   crossfilter,
   geneName
 ) {
-  const { varDataCache, worldObsIndex } = world;
+  const { worldObsIndex } = world;
   const varData = _worldVarDataCache[geneName];
   const worldIndex = worldObsIndex ? idx => worldObsIndex[idx] : idx => idx;
 
-  return crossfilter.dimension(r => {
-    return varData[worldIndex(r.__index__)];
-  }, Float32Array);
+  return crossfilter.dimension(
+    r => varData[worldIndex(r.__index__)],
+    Float32Array
+  );
 }
 
 export function createObsDimensionMap(crossfilter, world) {
@@ -317,7 +319,10 @@ export function createObsDimensionMap(crossfilter, world) {
     (result, anno) => {
       const dimType = deduceDimensionType(anno, anno.name);
       if (dimType) {
-        result[anno.name] = crossfilter.dimension(r => r[anno.name], dimType);
+        result[obsAnnoDimensionName(anno.name)] = crossfilter.dimension(
+          r => r[anno.name],
+          dimType
+        );
       } // else ignore the annotation
     },
     {}
@@ -327,11 +332,11 @@ export function createObsDimensionMap(crossfilter, world) {
   Add crossfilter dimensions allowing filtering on layout
   */
   const worldIndex = worldObsIndex ? idx => worldObsIndex[idx] : idx => idx;
-  dimensionMap.x = crossfilter.dimension(
+  dimensionMap[layoutDimensionName("X")] = crossfilter.dimension(
     r => obsLayout.X[worldIndex(r.__index__)],
     Float32Array
   );
-  dimensionMap.y = crossfilter.dimension(
+  dimensionMap[layoutDimensionName("Y")] = crossfilter.dimension(
     r => obsLayout.Y[worldIndex(r.__index__)],
     Float32Array
   );