do not hard-wire column names in annotations (#785)

* enforce column name uniqueness for obs and var

* parameterize the column name containing obs and var user-readable names

* use the new annotation index value from schema

* update f/e unit tests

* PR review suggestions

* lint

server/app/scanpy_engine/scanpy_engine.py

@@ -50,41 +50,61 @@ class ScanpyEngine(CXGDriver):
             "diffexp_lfc_cutoff": 0.01,
         }
 
-    def _alias_annotation_names(self, axis, name):
-        """
-        Do all user-specified annotation aliasing.
+    @staticmethod
+    def _create_unique_column_name(df, col_name_prefix):
+        """ given the columns of a dataframe, and a name prefix, return a column name which
+            does not exist in the dataframe, AND which is prefixed by `prefix`
 
-        As a *critical* side-effect, ensure the indices are simple number ranges
-        (accomplished by calling pandas.DataFrame.reset_index())
+            The approach is to append a numeric suffix, starting at zero and increasing by
+            one, until an unused name is found (eg, prefix_0, prefix_1, ...).
         """
-        if name == "name":
-            # a noop, so skip it
-            return
+        suffix = 0
+        while f"{col_name_prefix}{suffix}" in df:
+            suffix += 1
+        return f"{col_name_prefix}{suffix}"
 
-        ax_name = str(axis)
-        df_axis = getattr(self.data, ax_name)
-        if name is None:
-            # reset index to simple range; alias "name" to point at the
-            # previously specified index.
-            df_axis.reset_index(inplace=True)
-            df_axis.rename(inplace=True, columns={"index": "name"})
-        elif name in df_axis.columns:
-            if name not in df_axis.columns:
+    def _alias_annotation_names(self):
+        """
+        The front-end relies on the existance of a unique, human-readable
+        index for obs & var (eg, var is typically gene name, obs the cell name).
+        The user can specify these via the --obs-names and --var-names config.
+        If they are not specified, use the existing index to create them, giving
+        the resulting column a unique name (eg, "name").
+
+        In both cases, enforce that the result is unique, and communicate the
+        index column name to the front-end via the obs_names and var_names config
+        (which is incorporated into the schema).
+        """
+        for (ax_name, config_name) in ((Axis.OBS, "obs_names"), (Axis.VAR, "var_names")):
+            name = self.config[config_name]
+            df_axis = getattr(self.data, str(ax_name))
+            if name is None:
+                # Default: create unique names from index
+                if not df_axis.index.is_unique:
+                    raise KeyError(
+                        f"Values in {ax_name}.index must be unique. "
+                        "Please prepare data to contain unique index values, or specify an "
+                        "alternative with --{ax_name}-name."
+                    )
+                name = self._create_unique_column_name(df_axis.columns, "name_")
+                self.config[config_name] = name
+                # reset index to simple range; alias name to point at the
+                # previously specified index.
+                df_axis.rename_axis(name, inplace=True)
+                df_axis.reset_index(inplace=True)
+            elif name in df_axis.columns:
+                # User has specified alternative column for unique names, and it exists
+                if not df_axis[name].is_unique:
+                    raise KeyError(
+                        f"Values in {ax_name}.{name} must be unique. "
+                        "Please prepare data to contain unique values."
+                    )
+                df_axis.reset_index(drop=True, inplace=True)
+            else:
+                # user specified a non-existent column name
                 raise KeyError(
                     f"Annotation name {name}, specified in --{ax_name}-name does not exist."
                 )
-            if not df_axis[name].is_unique:
-                raise KeyError(
-                    f"Values in -{ax_name}-name must be unique. "
-                    "Please prepare data to contain unique values."
-                )
-            # reset index to simple range; alias user-specified annotation to "name"
-            df_axis.reset_index(drop=True, inplace=True)
-            df_axis.rename(inplace=True, columns={name: "name"})
-        else:
-            raise KeyError(
-                f"Annotation name {name}, specified in --{ax_name}_name does not exist."
-            )
 
     @staticmethod
     def _can_cast_to_float32(ann):
@@ -114,7 +134,16 @@ class ScanpyEngine(CXGDriver):
                 "nVar": self.gene_count,
                 "type": str(self.data.X.dtype),
             },
-            "annotations": {"obs": [], "var": []},
+            "annotations": {
+                "obs": {
+                    "index": self.config["obs_names"],
+                    "columns": []
+                },
+                "var": {
+                    "index": self.config["var_names"],
+                    "columns": []
+                }
+            },
             "layout": {"obs": []}
         }
         for ax in Axis:
@@ -139,7 +168,7 @@ class ScanpyEngine(CXGDriver):
                     raise TypeError(
                         f"Annotations of type {curr_axis[ann].dtype} are unsupported by cellxgene."
                     )
-                self.schema["annotations"][ax].append(ann_schema)
+                self.schema["annotations"][ax]["columns"].append(ann_schema)
 
         for layout in self.config['layout']:
             layout_schema = {
@@ -173,8 +202,11 @@ class ScanpyEngine(CXGDriver):
 
     @requires_data
     def _validate_and_initialize(self):
-        self._alias_annotation_names(Axis.OBS, self.config["obs_names"])
-        self._alias_annotation_names(Axis.VAR, self.config["var_names"])
+        # var and obs column names must be unique
+        if not self.data.obs.columns.is_unique or not self.data.var.columns.is_unique:
+            raise KeyError(f"All annotation column names must be unique.")
+
+        self._alias_annotation_names()
         self._validate_data_types()
         self.cell_count = self.data.shape[0]
         self.gene_count = self.data.shape[1]

server/test/schema.json

@@ -5,48 +5,54 @@
     "type": "float32"
   },
   "annotations": {
-    "obs": [
-      {
-        "name": "name",
-        "type": "string"
-      },
-      {
-        "name": "n_genes",
-        "type": "int32"
-      },
-      {
-        "name": "percent_mito",
-        "type": "float32"
-      },
-      {
-        "name": "n_counts",
-        "type": "float32"
-      },
-      {
-        "name": "louvain",
-        "type": "categorical",
-        "categories": [
-          "CD4 T cells",
-          "CD14+ Monocytes",
-          "B cells",
-          "CD8 T cells",
-          "NK cells",
-          "FCGR3A+ Monocytes",
-          "Dendritic cells",
-          "Megakaryocytes"
-        ]
-      }
-    ],
-    "var": [
-      {
-        "name": "name",
-        "type": "string"
-      },
-      {
-        "name": "n_cells",
-        "type": "int32"
-      }
-    ]
+    "obs": {
+      "index": "name_0",
+      "columns": [
+        {
+          "name": "name_0",
+          "type": "string"
+        },
+        {
+          "name": "n_genes",
+          "type": "int32"
+        },
+        {
+          "name": "percent_mito",
+          "type": "float32"
+        },
+        {
+          "name": "n_counts",
+          "type": "float32"
+        },
+        {
+          "name": "louvain",
+          "type": "categorical",
+          "categories": [
+            "CD4 T cells",
+            "CD14+ Monocytes",
+            "B cells",
+            "CD8 T cells",
+            "NK cells",
+            "FCGR3A+ Monocytes",
+            "Dendritic cells",
+            "Megakaryocytes"
+          ]
+        }
+      ]
+    },
+    "var": {
+      "index": "name_0",
+      "columns": [
+        {
+          "name": "name_0",
+          "type": "string"
+        },
+        {
+          "name": "n_cells",
+          "type": "int32"
+        }
+      ]
+    }
   },
   "layout": {
     "obs": [

server/test/test_api.py

@@ -23,7 +23,8 @@ class EndPoints(unittest.TestCase):
         session = requests.Session()
         for i in range(90):
             try:
-                session.get(f"{URL_BASE}schema")
+                result = session.get(f"{URL_BASE}schema")
+                cls.schema = result.json()
             except requests.exceptions.ConnectionError:
                 time.sleep(1)
 
@@ -45,7 +46,8 @@ class EndPoints(unittest.TestCase):
         self.assertEqual(result.headers["Content-Type"], "application/json")
         result_data = result.json()
         self.assertEqual(result_data["schema"]["dataframe"]["nObs"], 2638)
-        self.assertEqual(len(result_data["schema"]["annotations"]["obs"]), 5)
+        self.assertEqual(len(result_data["schema"]["annotations"]["obs"]), 2)
+        self.assertEqual(len(result_data["schema"]["annotations"]["obs"]["columns"]), 5)
 
     def test_config(self):
         endpoint = "config"
@@ -95,7 +97,8 @@ class EndPoints(unittest.TestCase):
         self.assertIsNotNone(df['col_idx'])
         self.assertIsNone(df['row_idx'])
         self.assertEqual(len(df['columns']), df['n_cols'])
-        self.assertListEqual(df['col_idx'], ['name', 'n_genes', 'percent_mito', 'n_counts', 'louvain'])
+        obs_index_col_name = self.schema["schema"]["annotations"]["obs"]["index"]
+        self.assertListEqual(df['col_idx'], [obs_index_col_name, 'n_genes', 'percent_mito', 'n_counts', 'louvain'])
 
     def test_get_annotations_obs_keys_fbs(self):
         endpoint = "annotations/obs"
@@ -165,7 +168,8 @@ class EndPoints(unittest.TestCase):
         self.assertIsNotNone(df['col_idx'])
         self.assertIsNone(df['row_idx'])
         self.assertEqual(len(df['columns']), df['n_cols'])
-        self.assertListEqual(df['col_idx'], ['name', 'n_cells'])
+        var_index_col_name = self.schema["schema"]["annotations"]["var"]["index"]
+        self.assertListEqual(df['col_idx'], [var_index_col_name, 'n_cells'])
 
     def test_get_annotations_var_keys_fbs(self):
         endpoint = "annotations/var"
@@ -247,7 +251,8 @@ class EndPoints(unittest.TestCase):
         endpoint = f"data/var"
         url = f"{URL_BASE}{endpoint}"
         header = {"Accept": "application/octet-stream"}
-        var_filter = {"filter": {"var": {"annotation_value": [{"name": "name", "values": ["RER1"]}]}}}
+        index_col_name = self.schema["schema"]["annotations"]["var"]["index"]
+        var_filter = {"filter": {"var": {"annotation_value": [{"name": index_col_name, "values": ["RER1"]}]}}}
         result = self.session.put(url, headers=header, json=var_filter)
         self.assertEqual(result.status_code, HTTPStatus.OK)
         self.assertEqual(result.headers["Content-Type"], "application/octet-stream")

server/test/test_nan_scanpy_engine.py

@@ -58,14 +58,16 @@ class NaNTest(unittest.TestCase):
 
     def test_annotation(self):
         annotations = decode_fbs.decode_matrix_FBS(self.data.annotation_to_fbs_matrix("obs"))
+        obs_index_col_name = self.data.schema["annotations"]["obs"]["index"]
         self.assertEqual(
             annotations["col_idx"],
-            ["name", "n_genes", "percent_mito", "n_counts", "louvain"]
+            [obs_index_col_name, "n_genes", "percent_mito", "n_counts", "louvain"]
         )
         self.assertEqual(annotations["n_rows"], 100)
         self.assertTrue(math.isnan(annotations["columns"][2][0]))
 
         annotations = decode_fbs.decode_matrix_FBS(self.data.annotation_to_fbs_matrix("var"))
-        self.assertEqual(annotations["col_idx"], ["name", "n_cells", "var_with_nans"])
+        var_index_col_name = self.data.schema["annotations"]["var"]["index"]
+        self.assertEqual(annotations["col_idx"], [var_index_col_name, "n_cells", "var_with_nans"])
         self.assertEqual(annotations["n_rows"], 100)
         self.assertTrue(math.isnan(annotations["columns"][2][0]))

server/test/test_scanpy_engine.py

@@ -31,9 +31,11 @@ class EngineTest(unittest.TestCase):
         self.assertTrue(self.data.data.X[0, 0] - -0.171_469_51 < epsilon)
 
     def test_mandatory_annotations(self):
-        self.assertIn("name", self.data.data.obs)
+        obs_index_col_name = self.data.schema["annotations"]["obs"]["index"]
+        self.assertIn(obs_index_col_name, self.data.data.obs)
         self.assertEqual(list(self.data.data.obs.index), list(range(2638)))
-        self.assertIn("name", self.data.data.var)
+        var_index_col_name = self.data.schema["annotations"]["var"]["index"]
+        self.assertIn(var_index_col_name, self.data.data.var)
         self.assertEqual(list(self.data.data.var.index), list(range(1838)))
 
     @pytest.mark.filterwarnings("ignore:Scanpy data matrix")
@@ -70,12 +72,14 @@ class EngineTest(unittest.TestCase):
         self.assertEqual(data["n_cols"], 91)
 
     def test_obs_and_var_names(self):
-        self.assertEqual(np.sum(self.data.data.var["name"].isna()), 0)
-        self.assertEqual(np.sum(self.data.data.obs["name"].isna()), 0)
+        self.assertEqual(np.sum(self.data.data.var[self.data.schema["annotations"]["var"]["index"]].isna()), 0)
+        self.assertEqual(np.sum(self.data.data.obs[self.data.schema["annotations"]["obs"]["index"]].isna()), 0)
 
     def test_schema(self):
         with open(path.join(path.dirname(__file__), "schema.json")) as fh:
             schema = json.load(fh)
+            print(schema)
+            print(self.data.schema)
             self.assertEqual(self.data.schema, schema)
 
     def test_schema_produces_error(self):
@@ -108,16 +112,18 @@ class EngineTest(unittest.TestCase):
         annotations = decode_fbs.decode_matrix_FBS(fbs)
         self.assertEqual(annotations["n_rows"], 2638)
         self.assertEqual(annotations["n_cols"], 5)
+        obs_index_col_name = self.data.schema["annotations"]["obs"]["index"]
         self.assertEqual(
             annotations["col_idx"],
-            ["name", "n_genes", "percent_mito", "n_counts", "louvain"],
+            [obs_index_col_name, "n_genes", "percent_mito", "n_counts", "louvain"],
         )
 
         fbs = self.data.annotation_to_fbs_matrix("var")
         annotations = decode_fbs.decode_matrix_FBS(fbs)
         self.assertEqual(annotations['n_rows'], 1838)
         self.assertEqual(annotations['n_cols'], 2)
-        self.assertEqual(annotations["col_idx"], ["name", "n_cells"])
+        var_index_col_name = self.data.schema["annotations"]["var"]["index"]
+        self.assertEqual(annotations["col_idx"], [var_index_col_name, "n_cells"])
 
     def test_annotation_fields(self):
         fbs = self.data.annotation_to_fbs_matrix("obs", ["n_genes", "n_counts"])
@@ -125,7 +131,8 @@ class EngineTest(unittest.TestCase):
         self.assertEqual(annotations["n_rows"], 2638)
         self.assertEqual(annotations['n_cols'], 2)
 
-        fbs = self.data.annotation_to_fbs_matrix("var", ["name"])
+        var_index_col_name = self.data.schema["annotations"]["var"]["index"]
+        fbs = self.data.annotation_to_fbs_matrix("var", [var_index_col_name])
         annotations = decode_fbs.decode_matrix_FBS(fbs)
         self.assertEqual(annotations['n_rows'], 1838)
         self.assertEqual(annotations['n_cols'], 1)
@@ -163,9 +170,10 @@ class EngineTest(unittest.TestCase):
             self.data.data_frame_to_fbs_matrix(filter_["filter"], "var")
 
     def test_data_named_gene(self):
+        var_index_col_name = self.data.schema["annotations"]["var"]["index"]
         filter_ = {
             "filter": {
-                "var": {"annotation_value": [{"name": "name", "values": ["RER1"]}]}
+                "var": {"annotation_value": [{"name": var_index_col_name, "values": ["RER1"]}]}
             }
         }
         fbs = self.data.data_frame_to_fbs_matrix(filter_["filter"], "var")
@@ -176,7 +184,7 @@ class EngineTest(unittest.TestCase):
 
         filter_ = {
             "filter": {
-                "var": {"annotation_value": [{"name": "name", "values": ["SPEN", "TYMP", "PRMT2"]}]}
+                "var": {"annotation_value": [{"name": var_index_col_name, "values": ["SPEN", "TYMP", "PRMT2"]}]}
             }
         }
         fbs = self.data.data_frame_to_fbs_matrix(filter_["filter"], "var")