remove --nan-to-num CLI parameter (#548)

* remove --nan-to-num CLI parameter

* factor tests better

* lint - remove unused variables

README.md

@@ -185,14 +185,6 @@ Currently this is not supported directly, but you should be able to do this manu
 - `.X` is used to display expression (histograms, scatterplot & colorscale) and to compute differential expression
 - `.obsm` is used for layout
 
-<hr>
-
-> When I start cellxgene, I get an error `Unexpected HTTP response 500, INTERNAL SERVER ERROR -- Out of range float values are not JSON compliant` in the web UI, or `Warning: JSON encoding failure - suggest trying --nan-to-num command line option` in the CLI. What can I do?
-
-At the moment, cellxgene is unable to transmit floating point NaN or Inifinty values to the web UI (due to a limitation on data serialization method in use). We expect to resolve this in a future release, but in the meantime, you can work around this issue by starting cellxgene with the `--nan-to-num` command line option, ie, `cellxgene launch data.h5ad --nan-to-num`.
-
-This option will convert all NaNs to zero, and all positive/negative infinities to the min/max of the data element within which the value was found (eg, +Infinity within an `obs` annotation will be converted to the maximum finite value in that annotation). This option will increase startup time, so we recommend only using it when the dataset contains NaN/Infinities.
-
 </details>
 
 <details>

docs/faq.md

@@ -86,12 +86,6 @@ cellxgene prepare data/ --output=data-processed.h5ad --recipe=zheng17
 
 It should be easy to run `prepare` then call `cellxgene launch` a few times with different settings to explore different behaviors. We may explore adding other preprocessing options in the future.
 
-#### When I start _cellxgene_, I get an error `Unexpected HTTP response 500, INTERNAL SERVER ERROR -- Out of range float values are not JSON compliant` in the web UI, or `Warning: JSON encoding failure - suggest trying --nan-to-num command line option` in the CLI. What can I do?
-
-At the moment, _cellxgene_ is unable to transmit floating point NaN or Infinity values to the web UI (due to a limitation on data serialization method in use). We expect to resolve this in a future release, but in the meantime, you can work around this issue by starting cellxgene with the `--nan-to-num` command line option, ie, `cellxgene launch data.h5ad --nan-to-num`.
-
-This option will convert all NaNs to zero, and all positive/negative infinities to the min/max of the data element within which the value was found (eg, +Infinity within an `obs` annotation will be converted to the maximum finite value in that annotation). This option will increase startup time, so we recommend only using it when the dataset contains NaN/Infinities.
-
 #### I tried to `pip install cellxgene` and got a weird error I don't understand
 
 This may happen, especially as we work out bugs in our installation process! Please create a new [Github issue](https://github.com/chanzuckerberg/cellxgene/issues), explain what you did, and include all the error messages you saw. It'd also be super helpful if you call `pip freeze` and include the full output alongside your issue.

server/app/scanpy_engine/scanpy_engine.py

@@ -42,10 +42,6 @@ class ScanpyEngine(CXGDriver):
         self.diffexp_options = ["ttest"]
         self._create_schema()
 
-        # TODO: temporary work-arounds
-        if args["nan_to_num"]:
-            self._IEEE754_special_values_workaround()
-
     def _alias_annotation_names(self, axis, name):
         """
         Do all user-specified annotation aliasing.
@@ -201,71 +197,6 @@ class ScanpyEngine(CXGDriver):
                 f"to solve this problem. "
             )
 
-    def _IEEE754_special_values_workaround(self):
-        """
-        TODO: temporary workaround
-
-        Because all floating point data is serialized to JSON, and JSON has no means of representing
-        non-finite, floating point special values (NaN, +/-Infinity, etc), we include this temporary
-        work-around.
-
-        This will likely be removed in the future, contingent upon improved marshalling.
-
-        Where non-finite floating point is present in obs, var or X:
-        * issue a warning to the user that these values will be convert to finite numbers.
-        * set NaN to zero, and Infinities to min/max of the element.
-        """
-
-        # annotations
-        for ax in Axis:
-            curr_axis = getattr(self.data, str(ax))
-            for ann in curr_axis:
-                dtype = curr_axis[ann].dtype
-                if dtype.kind == "f":
-                    finite_idx = np.isfinite(curr_axis[ann])
-                    if not finite_idx.all():
-                        curr_axis.loc[np.isnan(curr_axis[ann]), ann] = 0
-                        curr_axis.loc[np.isneginf(curr_axis[ann]), ann] = curr_axis[
-                            ann
-                        ][finite_idx].min()
-                        curr_axis.loc[np.isposinf(curr_axis[ann]), ann] = curr_axis[
-                            ann
-                        ][finite_idx].max()
-                        warnings.warn(
-                            f"{str(ax).title()} annotation '{ann}' contains floating point NaN or Infinities. "
-                            f"These will be converted to finite values."
-                        )
-
-        # X
-        non_finite_X_found = False
-        if sparse.issparse(self.data._X):
-            coo = self.data._X.tocoo()
-            finite_idx = np.isfinite(coo.data)
-            if not finite_idx.all():
-                non_finite_X_found = True
-                coo.data[np.isnan(coo.data)] = 0
-                coo.data[np.isneginf(coo.data)] = np.min(coo.data[finite_idx])
-                coo.data[np.isposinf(coo.data)] = np.max(coo.data[finite_idx])
-                coo.eliminate_zeros()
-                _X = coo.asformat(self.data._X.getformat())
-                self.data._X = _X
-        else:
-            _X = self.data._X
-            finite_idx = np.isfinite(_X.flat)
-            if not finite_idx.all():
-                non_finite_X_found = True
-                min_X = _X.flat[finite_idx].min()
-                max_X = _X.flat[finite_idx].max()
-                _X[np.isnan(_X)] = 0
-                _X[np.isneginf(_X)] = min_X
-                _X[np.isposinf(_X)] = max_X
-
-        if non_finite_X_found:
-            warnings.warn(
-                "Dataframe X contains floating point NaN or Infinities. "
-                "These will be converted to finite values."
-            )
-
     def filter_dataframe(self, filter):
         """
          Filter cells from data and return a subset of the data. They can operate on both obs and var dimension with
@@ -480,7 +411,9 @@ class ScanpyEngine(CXGDriver):
             raise FilterError("filtering on obs unsupported")
 
         # Currently only handles VAR dimension
-        X = self.data._X[:, var_selector]
+        X = self.data._X
+        if var_selector is not None:
+            X = X[:, var_selector]
         return encode_matrix_fbs(X, col_idx=np.nonzero(var_selector)[0], row_idx=None)
 
     def diffexp_topN(self, obsFilterA, obsFilterB, top_n=None, interactive_limit=None):

server/app/util/constants.py

@@ -28,5 +28,5 @@ class DiffExpMode(AugmentedEnum):
 
 
 JSON_NaN_to_num_warning_msg = (
-    "JSON encoding failure - suggest trying --nan-to-num command line option"
+    "JSON encoding failure - please verify all data are finite values (no NaN or Infinities)"
 )

server/cli/launch.py

@@ -60,13 +60,6 @@ from server.app.util.utils import custom_format_warning
     show_default=True,
     help="Relative expression cutoff used when selecting top N differentially expressed genes",
 )
-@click.option(
-    "--nan-to-num",
-    is_flag=True,
-    default=False,
-    show_default=True,
-    help="Replace all floating point NaN with zero, and infinities with finite numbers",
-)
 def launch(
     data,
     layout,
@@ -81,7 +74,6 @@ def launch(
     host,
     max_category_items,
     diffexp_lfc_cutoff,
-    nan_to_num,
 ):
     """Launch the cellxgene data viewer.
     This web app lets you explore single-cell expression data.
@@ -143,7 +135,6 @@ def launch(
         "diffexp_lfc_cutoff": diffexp_lfc_cutoff,
         "obs_names": obs_names,
         "var_names": var_names,
-        "nan_to_num": nan_to_num,
     }
 
     try:

server/test/test_nan_rest.py

@@ -49,48 +49,3 @@ class WithNaNs(unittest.TestCase):
             url = f"{URL_BASE}{endpoint}"
             result = self.session.get(url)
             self.assertEqual(result.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
-
-
-class WithoutNaNs(unittest.TestCase):
-    """Test Case for endpoints"""
-
-    @classmethod
-    def setUpClass(cls):
-        cls.ps = Popen(
-            [
-                "cellxgene",
-                "launch",
-                "server/test/test_datasets/nan.h5ad",
-                "--nan-to-num",
-                "--debug",
-            ]
-        )
-        session = requests.Session()
-        for i in range(90):
-            try:
-                session.get(f"{URL_BASE}schema")
-            except requests.exceptions.ConnectionError:
-                time.sleep(1)
-
-    @classmethod
-    def tearDownClass(cls):
-        try:
-            cls.ps.terminate()
-        except ProcessLookupError:
-            pass
-
-    def setUp(self):
-        self.session = requests.Session()
-
-    def test_initialize(self):
-        endpoint = "schema"
-        url = f"{URL_BASE}{endpoint}"
-        result = self.session.get(url)
-        self.assertEqual(result.status_code, HTTPStatus.OK)
-
-    def test_errors(self):
-        endpoints = ["annotations/obs", "annotations/var", "data/obs", "data/var"]
-        for endpoint in endpoints:
-            url = f"{URL_BASE}{endpoint}"
-            result = self.session.get(url)
-            self.assertEqual(result.status_code, HTTPStatus.OK)

server/test/test_nan_scanpy_engine.py

@@ -2,6 +2,9 @@ import json
 import pytest
 import unittest
 import warnings
+import math
+
+import decode_fbs
 
 from server.app.scanpy_engine.scanpy_engine import ScanpyEngine
 from server.app.util.errors import JSONEncodingValueError
@@ -16,24 +19,15 @@ class NaNTest(unittest.TestCase):
             "obs_names": None,
             "var_names": None,
             "diffexp_lfc_cutoff": 0.01,
-            "nan_to_num": False,
         }
         with warnings.catch_warnings():
             warnings.simplefilter("ignore", category=UserWarning)
             self.data = ScanpyEngine("server/test/test_datasets/nan.h5ad", self.args)
             self.data._create_schema()
-        self.args_nan = dict(self.args)
-        self.args_nan["nan_to_num"] = True
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", category=UserWarning)
-            self.data_nan = ScanpyEngine(
-                "server/test/test_datasets/nan.h5ad", self.args_nan
-            )
-            self.data_nan._create_schema()
 
     def test_load(self):
         with self.assertWarns(UserWarning):
-            ScanpyEngine("server/test/test_datasets/nan.h5ad", self.args_nan)
+            ScanpyEngine("server/test/test_datasets/nan.h5ad", self.args)
 
     def test_init(self):
         self.assertEqual(self.data.cell_count, 100)
@@ -41,45 +35,38 @@ class NaNTest(unittest.TestCase):
         epsilon = 0.000_005
         self.assertTrue(self.data.data.X[0, 0] - -0.171_469_51 < epsilon)
 
-        self.assertEqual(self.data_nan.cell_count, 100)
-        self.assertEqual(self.data_nan.gene_count, 100)
-        epsilon = 0.000_005
-        self.assertTrue(self.data_nan.data.X[0, 0] - -0.171_469_51 < epsilon)
-
     def test_dataframe(self):
-        data_frame_obs = json.loads(self.data_nan.data_frame(None, "obs"))
-        self.assertEqual(len(data_frame_obs["var"]), 100)
-        self.assertEqual(len(data_frame_obs["obs"]), 100)
-        data_frame_var = json.loads(self.data_nan.data_frame(None, "var"))
-        self.assertEqual(len(data_frame_var["var"]), 100)
-        self.assertEqual(len(data_frame_var["obs"]), 100)
-        with pytest.raises(JSONEncodingValueError):
-            data_frame_obs = json.loads(self.data.data_frame(None, "obs"))
-        with pytest.raises(JSONEncodingValueError):
-            data_frame_var = json.loads(self.data.data_frame(None, "var"))
+        data_frame_var = decode_fbs.decode_matrix_FBS(self.data.data_frame_to_fbs_matrix(None, "var"))
+        self.assertIsNotNone(data_frame_var)
+        self.assertEqual(data_frame_var["n_rows"], 100)
+        self.assertEqual(data_frame_var["n_cols"], 100)
+        self.assertTrue(math.isnan(data_frame_var["columns"][3][3]))
 
-    def test_dataframe_nan_to_0(self):
-        data_frame_obs = json.loads(self.data_nan.data_frame(None, "obs"))
-        self.assertEqual(data_frame_obs["obs"][1][3], 0.0)
-        data_frame_var = json.loads(self.data_nan.data_frame(None, "var"))
-        self.assertEqual(data_frame_var["var"][1][5], 0.0)
+        with pytest.raises(JSONEncodingValueError):
+            json.loads(self.data.data_frame(None, "obs"))
+        with pytest.raises(JSONEncodingValueError):
+            json.loads(self.data.data_frame(None, "var"))
 
-    def test_annotation_nan_to_0(self):
-        annotations_obs = json.loads(self.data_nan.annotation(None, "obs"))
-        self.assertEqual(annotations_obs["data"][0][3], 0.0)
-        annotations_var = json.loads(self.data_nan.annotation(None, "var"))
-        self.assertEqual(annotations_var["data"][0][3], 0.0)
+    def test_dataframe_obs_not_implemented(self):
+        with self.assertRaises(ValueError) as cm:
+            decode_fbs.decode_matrix_FBS(self.data.data_frame_to_fbs_matrix(None, "obs"))
+        self.assertIsNotNone(cm.exception)
 
     def test_annotation(self):
-        annotations = json.loads(self.data_nan.annotation(None, "obs"))
+        annotations = decode_fbs.decode_matrix_FBS(self.data.annotation_to_fbs_matrix("obs"))
         self.assertEqual(
-            annotations["names"],
-            ["name", "n_genes", "percent_mito", "n_counts", "louvain"],
+            annotations["col_idx"],
+            ["name", "n_genes", "percent_mito", "n_counts", "louvain"]
         )
-        annotations = json.loads(self.data_nan.annotation(None, "var"))
-        self.assertEqual(annotations["names"], ["name", "n_cells", "var_with_nans"])
-        self.assertEqual(len(annotations["data"]), 100)
+        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"])
+        self.assertEqual(annotations["n_rows"], 100)
+        self.assertTrue(math.isnan(annotations["columns"][2][0]))
+
         with pytest.raises(JSONEncodingValueError):
-            annotations = json.loads(self.data.annotation(None, "obs"))
+            json.loads(self.data.annotation(None, "obs"))
         with pytest.raises(JSONEncodingValueError):
-            annotations = json.loads(self.data.annotation(None, "var"))
+            json.loads(self.data.annotation(None, "var"))

server/test/test_scanpy_engine.py

@@ -19,7 +19,6 @@ class UtilTest(unittest.TestCase):
             "obs_names": None,
             "var_names": None,
             "diffexp_lfc_cutoff": 0.01,
-            "nan_to_num": True,
         }
 
         self.data = ScanpyEngine("example-dataset/pbmc3k.h5ad", args)