improve handling of non-finite floating point values (#490)

* improve HTTP error reporting

* generate standards-compatible JSON

* add --nan-to-num work-around for non-finite floating point values

* lint

* update tests

* correctly set Infinities to min/max

* REAMDE update for --nan-to-num

* define constant for repetitive warning message

* clarify where NaN errors will occure

README.md

@@ -27,6 +27,7 @@ If you want an example dataset download [this file](https://github.com/chanzucke
 ```
 cellxgene launch pbmc3k.h5ad --open
 ```
+
 You should see your web browser open with the following
 
 <img width="450" src="https://raw.githubusercontent.com/chanzuckerberg/cellxgene/master/docs/cellxgene-opening-screenshot.png" pad="50px">
@@ -35,7 +36,7 @@ You should see your web browser open with the following
 
 There are several options available, such as:
 
-- `--layout` to specify the layout as  `tsne` or `umap`
+- `--layout` to specify the layout as `tsne` or `umap`
 - `--title` to show a title on the explorer
 - `--open` to automatically open the web browser after launching (OS X only)
 
@@ -56,11 +57,11 @@ The `launch` command assumes that the data is stored in the `.h5ad` format from
 - an `obs` field has a unique identifier for every cell (you can specify which field to use with the `--obs-names` option, by default it will use the value of `data.obs_names`)
 - a `var` field has a unique identifier for every gene (you can specify which field to use with the `--var-names` option, by default it will use the value of `data.var_names`)
 - an `obsm` field contains the two-dimensional coordinates for the layout that you want to render (e.g. `X_tsne` for the `tsne` layout or `X_umap` for the `umap` layout)
-- any additional  `obs` fields will be rendered as per-cell continuous or categorical metadata by the app (e.g. `louvain` cluster assignments) 
+- any additional `obs` fields will be rendered as per-cell continuous or categorical metadata by the app (e.g. `louvain` cluster assignments)
 
 ### prepare
 
-The `prepare` command is included to help you format your data. It uses `scanpy` under the hood. This is especially useful if you are starting with raw unanalyzed data and are unfamiliar with `scanpy`. 
+The `prepare` command is included to help you format your data. It uses `scanpy` under the hood. This is especially useful if you are starting with raw unanalyzed data and are unfamiliar with `scanpy`.
 
 To prepare from an existing `.h5ad` file use
 
@@ -127,7 +128,7 @@ And your web browser should open with an interactive view of your data.
 
 > In my `prepare` command I received the following error `Warning: louvain module is not installed, no clusters will be calculated. To fix this please install cellxgene with the optional feature louvain enabled`
 
-Louvain clustering requires additional dependencies that are somewhat complex, so we don't include them by default. For now, you need to specify that you want these packages by using 
+Louvain clustering requires additional dependencies that are somewhat complex, so we don't include them by default. For now, you need to specify that you want these packages by using
 
 ```
 pip install cellxgene[louvain]
@@ -153,7 +154,7 @@ This may happen, especially as we work out bugs in our installation process! Ple
 
 > How are you computing and sorting differential expression results?
 
-Currently we use a [Welch's *t*-test](https://en.wikipedia.org/wiki/Welch%27s_t-test) implementation including the same variance overestimation correction as used in `scanpy`. We sort the `tscore` to identify the top N genes, and then filter to remove any that fall below a cutoff log fold change value, which can help remove spurious test results. The default threshold is `0.01` and can be changed using the option `--diffexp-lfc-cutoff`. We can explore adding support for other test types in the future.
+Currently we use a [Welch's _t_-test](https://en.wikipedia.org/wiki/Welch%27s_t-test) implementation including the same variance overestimation correction as used in `scanpy`. We sort the `tscore` to identify the top N genes, and then filter to remove any that fall below a cutoff log fold change value, which can help remove spurious test results. The default threshold is `0.01` and can be changed using the option `--diffexp-lfc-cutoff`. We can explore adding support for other test types in the future.
 
 > I'm following the developer instructions and get an error about "missing files and directories” when trying to build the client
 
@@ -165,6 +166,12 @@ This is likely because you do not have node and npm installed, we recommend usin
 - `.X` is used to display expression (histograms, scatterplot & colorscale) and to compute differential expression
 - `.obsm` is used for layout
 
+> 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.
+
 ## developer guide
 
 This project has made a few key design choices
@@ -204,11 +211,11 @@ If you have any questions about developing or contributing, come hang out with u
 
 ## development roadmap
 
-`cellxgene` is still very much in development, and we've love to include the community as we plan new features to work on. We are thinking about working on the following features over the next 3-12 months. If you are interested in updates, want to give feedback, want to contribute, or have ideas about other features we should work on, please [contact us](#help-and-contact) 
+`cellxgene` is still very much in development, and we've love to include the community as we plan new features to work on. We are thinking about working on the following features over the next 3-12 months. If you are interested in updates, want to give feedback, want to contribute, or have ideas about other features we should work on, please [contact us](#help-and-contact)
 
 - **Visualizaling spatial metadata** Image-based transcriptomics methods also generate large cell by gene matrices, alongside rich metadata about spatial location; we would like to render this information in `cellxgene`
-- **Visualizing trajectories** Trajectory analyses infer progression along some ordering or pseudotime; we would like `cellxgene ` to render the results of these analyses when they have been performed
-- **Deploy to web**  Many projects release public data browser websites  alongside their publicatons; we would like to make it easy for anyone to deploy `cellxgene` to a custom URL with their own dataset that they own and operate
+- **Visualizing trajectories** Trajectory analyses infer progression along some ordering or pseudotime; we would like `cellxgene` to render the results of these analyses when they have been performed
+- **Deploy to web** Many projects release public data browser websites alongside their publicatons; we would like to make it easy for anyone to deploy `cellxgene` to a custom URL with their own dataset that they own and operate
 - **HCA Integration** The [Human Cell Atlas](https://humancellatlas.org) is generating a large corpus of single-cell expression data and will make it available through the Data Coordination Platform; we would like `cellxgene` to be one of several different portals for browsing these data
 
 ## contributing
@@ -221,7 +228,7 @@ We've been heavily inspired by several other related single-cell visualization p
 
 We were inspired by Mike Bostock and the [crossfilter](https://github.com/crossfilter) team for the design of our filtering implementation.
 
-We have been working closely with the [`scanpy`](https://github.com/theislab/scanpy) team to integrate with their awesome analysis tools. Special thanks to Alex Wolf, Fabian Theis, and the rest of the team for their help during development and for providing an example dataset. 
+We have been working closely with the [`scanpy`](https://github.com/theislab/scanpy) team to integrate with their awesome analysis tools. Special thanks to Alex Wolf, Fabian Theis, and the rest of the team for their help during development and for providing an example dataset.
 
 We are eager to explore integrations with other computational backends such as [`Seurat`](https://github.com/satijalab/seurat) or [`Bioconductor`](https://github.com/Bioconductor)
 

client/src/util/actionHelpers.js

@@ -38,7 +38,11 @@ export const doJsonRequest = async url => {
     return res.json();
   }
   // else an error
-  const msg = `Unexpected HTTP response ${res.status}, ${res.statusText}`;
+  let msg = `Unexpected HTTP response ${res.status}, ${res.statusText}`;
+  const body = await res.text();
+  if (body && body.length > 0) {
+    msg = `${msg} -- ${body}`;
+  }
   dispatchNetworkErrorMessageToUser(msg);
   throw new Error(msg);
 };

server/app/rest_api/rest.py

@@ -1,5 +1,6 @@
 from http import HTTPStatus
 import pkg_resources
+import warnings
 
 from flask import (
     Blueprint, current_app, jsonify, make_response, request
@@ -7,7 +8,7 @@ from flask import (
 from flask_restful_swagger_2 import Api, swagger, Resource
 from werkzeug.datastructures import ImmutableMultiDict
 
-from server.app.util.constants import Axis, DiffExpMode
+from server.app.util.constants import Axis, DiffExpMode, JSON_NaN_to_num_warning_msg
 from server.app.util.filter import parse_filter, QueryStringError
 from server.app.util.models import FilterModel
 from server.app.util.utils import get_mime_type
@@ -160,7 +161,12 @@ class AnnotationsObsAPI(Resource):
             annotation_response = current_app.data.annotation({}, "obs", fields)
         except KeyError:
             return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
-        return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        try:
+            return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @swagger.doc({
         "summary": "Fetch annotations (metadata) for filtered subset of observations.",
@@ -211,7 +217,12 @@ class AnnotationsObsAPI(Resource):
             return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
         except FilterError as e:
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
-        return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        try:
+            return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 class AnnotationsVarAPI(Resource):
@@ -253,7 +264,12 @@ class AnnotationsVarAPI(Resource):
             annotation_response = current_app.data.annotation({}, "var", fields)
         except KeyError:
             return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
-        return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        try:
+            return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @swagger.doc({
         "summary": "Fetch annotations (metadata) for filtered subset of variables.",
@@ -302,7 +318,12 @@ class AnnotationsVarAPI(Resource):
             return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
         except FilterError:
             return make_response("Malformed filter", HTTPStatus.BAD_REQUEST)
-        return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        try:
+            return make_response(jsonify(annotation_response), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 class DataObsAPI(Resource):
@@ -363,6 +384,10 @@ class DataObsAPI(Resource):
             return make_response((jsonify(current_app.data.data_frame(filter_, axis=Axis.OBS))), HTTPStatus.OK)
         except FilterError as e:
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @swagger.doc({
         "summary": "Get data (expression values) from the dataframe.",
@@ -407,6 +432,10 @@ class DataObsAPI(Resource):
                                  HTTPStatus.OK)
         except FilterError as e:
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 class DataVarAPI(Resource):
@@ -465,6 +494,10 @@ class DataVarAPI(Resource):
             return make_response((jsonify(current_app.data.data_frame(filter_, axis=Axis.VAR))), HTTPStatus.OK)
         except FilterError as e:
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @swagger.doc({
         "summary": "Get data (expression values) from the dataframe.",
@@ -510,6 +543,10 @@ class DataVarAPI(Resource):
                                  HTTPStatus.OK)
         except FilterError as e:
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 class DiffExpObsAPI(Resource):
@@ -614,7 +651,12 @@ class DiffExpObsAPI(Resource):
             return make_response(e.message, HTTPStatus.BAD_REQUEST)
         except InteractiveError:
             return make_response("Non-interactive request", HTTPStatus.FORBIDDEN)
-        return make_response(jsonify(diffexp), HTTPStatus.OK)
+        try:
+            return make_response(jsonify(diffexp), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 class LayoutObsAPI(Resource):
@@ -647,7 +689,12 @@ class LayoutObsAPI(Resource):
             layout = current_app.data.layout({})
         except PrepareError as e:
             return make_response(e.message, HTTPStatus.INTERNAL_SERVER_ERROR)
-        return make_response((jsonify({"layout": layout})), HTTPStatus.OK)
+        try:
+            return make_response((jsonify({"layout": layout})), HTTPStatus.OK)
+        except ValueError as e:
+            # JSON encoding failure, usually due to bad data
+            warnings.warn(JSON_NaN_to_num_warning_msg)
+            return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
 
     # @swagger.doc({
     #     "summary": "Observation layout for filtered subset.",

server/app/scanpy_engine/scanpy_engine.py

@@ -36,8 +36,8 @@ class ScanpyEngine(CXGDriver):
         self._create_schema()
 
         # TODO: temporary work-arounds
-        self._IEEE754_X_warning_issued = False
-        self._IEEE754_special_values_workaround_annotations()
+        if args['nan_to_num']:
+            self._IEEE754_special_values_workaround()
 
     def _alias_annotation_names(self, axis, name):
         """
@@ -176,7 +176,7 @@ class ScanpyEngine(CXGDriver):
                 f"`cellxgene prepare --layout {self.layout_method} <datafile>` "
                 f"to solve this problem. ")
 
-    def _IEEE754_special_values_workaround_annotations(self):
+    def _IEEE754_special_values_workaround(self):
         """
         TODO: temporary workaround
 
@@ -187,40 +187,55 @@ class ScanpyEngine(CXGDriver):
         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 treated as zeros.
-        * set the value to zero within the in-memory data (self.data)
+        * 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':
-                    not_finite = np.isfinite(curr_axis[ann]) == False   # noqa: E712
-                    if np.count_nonzero(not_finite) > 0:
+                    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 values will be treated as zero."
+                            f"These will be converted to finite values."
                         )
-                        curr_axis[ann][not_finite] = 0
 
-    def _IEEE754_special_values_workaround_X(self, _X):
-        """
-        TODO: temporary workaround
+        # 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
 
-        See comments in _IEEE754_special_values_workaround_annotations
-        """
-        not_finite = np.isfinite(_X) == False   # noqa: E712
-        if np.count_nonzero(not_finite) > 0:
-            _X[not_finite] = 0
-            if not self._IEEE754_X_warning_issued:
-                # only want to issue this warning once.
-                warnings.warn(
-                    "Dataframe X contains floating point NaN or Infinities. "
-                    "These values will be treated as zero."
-                )
-                self._IEEE754_X_warning_issued = True
-
-        return _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):
         """
@@ -373,7 +388,6 @@ class ScanpyEngine(CXGDriver):
             _X = _X.toarray()
         var_index_sliced = self.data.var.index[var_selector]
         obs_index_sliced = self.data.obs.index[obs_selector]
-        _X = self._IEEE754_special_values_workaround_X(_X)
         if axis == Axis.OBS:
             result = {
                 "var": var_index_sliced.tolist(),

server/app/util/constants.py

@@ -25,3 +25,6 @@ class Axis(AugmentedEnum):
 class DiffExpMode(AugmentedEnum):
     TOP_N = "topN"
     VAR_FILTER = "varFilter"
+
+
+JSON_NaN_to_num_warning_msg = "JSON encoding failure - suggest trying --nan-to-num command line option"

server/app/util/utils.py

@@ -7,6 +7,17 @@ from server.app.util.errors import MimeTypeError
 
 
 class Float32JSONEncoder(json.JSONEncoder):
+    def __init__(self, *args, **kwargs):
+        """
+        NaN/Infinities are illegal in standard JSON.  Python extends JSON with
+        non-standard symbols that most JavaScript JSON parsers do not understand.
+        The `allow_nan` parameter will force Python simplejson to throw an ValueError
+        if it runs into non-finite floating point values which are unsupported by
+        standard JSON.
+        """
+        kwargs['allow_nan'] = False
+        super().__init__(*args, **kwargs)
+
     def default(self, obj):
         if isinstance(obj, float32):
             return float(obj)

server/cli/launch.py

@@ -29,8 +29,11 @@ from server.app.util.errors import ScanpyFileError
               help="Limits the number of categorical annotation items displayed.")
 @click.option("--diffexp-lfc-cutoff", default=0.01, 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, diffexp, title, verbose, debug, obs_names, var_names,
-           open_browser, port, host, max_category_items, diffexp_lfc_cutoff):
+           open_browser, port, 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.
     Data must be in a format that cellxgene expects, read the
@@ -91,7 +94,8 @@ def launch(data, layout, diffexp, title, verbose, debug, obs_names, var_names,
         "max_category_items": max_category_items,
         "diffexp_lfc_cutoff": diffexp_lfc_cutoff,
         "obs_names": obs_names,
-        "var_names": var_names
+        "var_names": var_names,
+        "nan_to_num": nan_to_num
     }
 
     try:

server/test/test_scanpy_engine.py

@@ -14,7 +14,8 @@ from server.app.scanpy_engine.scanpy_engine import ScanpyEngine
 class UtilTest(unittest.TestCase):
     def setUp(self):
         args = {'layout': 'umap', 'diffexp': 'ttest', 'max_category_items': 100,
-                'obs_names': None, 'var_names': None, 'diffexp_lfc_cutoff': 0.01}
+                'obs_names': None, 'var_names': None, 'diffexp_lfc_cutoff': 0.01,
+                'nan_to_num': True}
 
         self.data = ScanpyEngine("example-dataset/pbmc3k.h5ad", args)
         self.data._create_schema()