Do not calculate layout, used saved layout instead (#343)

* Do not calculate layout, used saved layout instead

See for rationale: https://docs.google.com/document/d/1HJFvbdDHxxgkCW0DZzdTZ9CUMgc2ef2rQFukATBQWvE/edit

* Error handling for when layout has not been precomputed

* Server error (500) not client error (400) for unprepared data

docs/REST_API.md

@@ -460,8 +460,12 @@ Get the _default_ layout for all observations or (_future_) all variables. Retur
   }
 }
 ```
+**Response code:**
 
-### PUT /layout/obs, (_future_) PUT /layout/var
+- 200 - success
+- 500 Internal Server Error - unprepared data, layout has not been stored in the input data
+
+### (_future_) PUT /layout/obs, (_future_) PUT /layout/var
 
 Generate layout for the caller-specified subset of data, as indicated by the filter. This operation implicitly requests a re-layout operation to be performed on the specified data. This operation will _commonly_ return the same results for any given caller-specified filter, but this behavior is not guaranteed.
 

server/app/rest_api/rest.py

@@ -10,7 +10,7 @@ from werkzeug.datastructures import ImmutableMultiDict
 from server.app.util.constants import Axis, DiffExpMode
 from server.app.util.filter import parse_filter, QueryStringError
 from server.app.util.models import FilterModel
-from server.app.util.utils import FilterError, InteractiveError, MimeTypeError, get_mime_type
+from server.app.util.utils import FilterError, InteractiveError, MimeTypeError, PrepareError, get_mime_type
 
 """
 Sort order for routes
@@ -630,56 +630,63 @@ class LayoutObsAPI(Resource):
                         }
                     }
                 }
+            },
+            "400": {
+                "description": "Data preparation error"
             }
         }
     })
     def get(self):
-        return make_response((jsonify({"layout": current_app.data.layout({})})), HTTPStatus.OK)
-
-    @swagger.doc({
-        "summary": "Observation layout for filtered subset.",
-        "tags": ["layout"],
-        "parameters": [
-            {
-                "name": "filter",
-                "description": "Complex Filter",
-                "in": "body",
-                "schema": FilterModel
-            }
-        ],
-        "responses": {
-            "200": {
-                "description": "layout",
-                "examples": {
-                    "application/json": {
-                        "layout": {
-                            "ndims": 2,
-                            "coordinates": [
-                                [0, 0.284483, 0.983744],
-                                [1, 0.038844, 0.739444]
-                            ]
-                        }
-                    }
-                }
-            },
-            "400": {
-                "description": "Malformed filter"
-            },
-            "403": {
-                "description": "Non-interactive request"
-            },
-        }
-    })
-    def put(self):
         try:
-            filter = request.get_json()["filter"]
+            layout = current_app.data.layout({})
-            interactive_limit = current_app.data.features["layout"]["obs"]["interactiveLimit"]
+        except PrepareError as e:
-            layout = current_app.data.layout(filter, interactive_limit=interactive_limit)
+            return make_response(e.message, HTTPStatus.INTERNAL_SERVER_ERROR)
-            return make_response(jsonify({"layout": layout}), HTTPStatus.OK)
+        return make_response((jsonify({"layout": layout})), HTTPStatus.OK)
-        except FilterError as e:
+
-            return make_response(e.message, HTTPStatus.BAD_REQUEST)
+    # @swagger.doc({
-        except InteractiveError:
+    #     "summary": "Observation layout for filtered subset.",
-            return make_response("Non-interactive request", HTTPStatus.FORBIDDEN)
+    #     "tags": ["layout"],
+    #     "parameters": [
+    #         {
+    #             "name": "filter",
+    #             "description": "Complex Filter",
+    #             "in": "body",
+    #             "schema": FilterModel
+    #         }
+    #     ],
+    #     "responses": {
+    #         "200": {
+    #             "description": "layout",
+    #             "examples": {
+    #                 "application/json": {
+    #                     "layout": {
+    #                         "ndims": 2,
+    #                         "coordinates": [
+    #                             [0, 0.284483, 0.983744],
+    #                             [1, 0.038844, 0.739444]
+    #                         ]
+    #                     }
+    #                 }
+    #             }
+    #         },
+    #         "400": {
+    #             "description": "Malformed filter"
+    #         },
+    #         "403": {
+    #             "description": "Non-interactive request"
+    #         },
+    #     }
+    # })
+    # def put(self):
+    #     try:
+    #         filter = request.get_json()["filter"]
+    #         interactive_limit = current_app.data.features["layout"]["obs"]["interactiveLimit"]
+    #         layout = current_app.data.layout(filter, interactive_limit=interactive_limit)
+    #         return make_response(jsonify({"layout": layout}), HTTPStatus.OK)
+    #     except FilterError as e:
+    #         return make_response(e.message, HTTPStatus.BAD_REQUEST)
+    #     except InteractiveError:
+    #         return make_response("Non-interactive request", HTTPStatus.FORBIDDEN)
 
 
 def get_api_resources():

server/app/scanpy_engine/scanpy_engine.py

@@ -9,7 +9,7 @@ from scipy import stats
 from server.app.app import cache
 from server.app.driver.driver import CXGDriver
 from server.app.util.constants import Axis, DEFAULT_TOP_N, DiffExpMode
-from server.app.util.utils import FilterError, InteractiveError
+from server.app.util.utils import FilterError, InteractiveError, PrepareError
 
 """
 Sort order for methods
@@ -30,7 +30,6 @@ class ScanpyEngine(CXGDriver):
         self.cell_count = self.data.shape[0]
         self.gene_count = self.data.shape[1]
         self._create_schema()
-        self.layout({})
 
     def _create_schema(self):
         self.schema = {
@@ -340,8 +339,14 @@ class ScanpyEngine(CXGDriver):
         # TODO Filtering cells is fine, but filtering genes does nothing because the neighbors are
         # calculated using the original vars (geneset) and this doesn’t get updated when you use less.
         # Need to recalculate neighbors (long) if user requests new layout filtered by var
-        getattr(sc.tl, self.layout_method)(df, random_state=123)
+        # TODO for MVP we are pushing computation of layout to preprocessing and not allowing re-layout
-        df_layout = df.obsm[f"X_{self.layout_method}"]
+        # this will probably change after user feedback
+        # getattr(sc.tl, self.layout_method)(df, random_state=123)
+        try:
+            df_layout = df.obsm[f"X_{self.layout_method}"]
+        except ValueError as e:
+            raise PrepareError(f"Layout has not been calculated using {self.layout_method}, "
+                               f"please prepare your datafile and relaunch cellxgene") from e
         normalized_layout = DataFrame((df_layout - df_layout.min()) / (df_layout.max() - df_layout.min()),
                                       index=df.obs.index)
         return {

server/app/util/utils.py

@@ -30,6 +30,12 @@ class InteractiveError(Exception):
         self.message = message
 
 
+class PrepareError(Exception):
+
+    def __init__(self, message):
+        self.message = message
+
+
 def get_mime_type(default="application/json", acceptable_types=["application/json", "text/csv"], query_param=None,
                   header=None):
     mime_type = default

server/test/test_api.py

@@ -70,27 +70,27 @@ class EndPoints(unittest.TestCase):
         self.assertEqual(result_data["layout"]["ndims"], 2)
         self.assertEqual(len(result_data["layout"]["coordinates"]), 2638)
 
-    def test_put_layout(self):
+    # def test_put_layout(self):
-        endpoint = "layout/obs"
+    #     endpoint = "layout/obs"
-        url = f"{URL_BASE}{endpoint}"
+    #     url = f"{URL_BASE}{endpoint}"
-        obs_filter = {
+    #     obs_filter = {
-            "filter": {
+    #         "filter": {
-                "obs": {
+    #             "obs": {
-                    "annotation_value": [
+    #                 "annotation_value": [
-                        {"name": "louvain", "values": ["NK cells", "CD8 T cells"]},
+    #                     {"name": "louvain", "values": ["NK cells", "CD8 T cells"]},
-                        {"name": "n_counts", "min": 3000},
+    #                     {"name": "n_counts", "min": 3000},
-                    ],
+    #                 ],
-                    "index": [1, 99, [1000, 2000]]
+    #                 "index": [1, 99, [1000, 2000]]
-                }
+    #             }
-            }
+    #         }
-        }
+    #     }
-        result = self.session.put(url, json=obs_filter)
+    #     result = self.session.put(url, json=obs_filter)
-        self.assertEqual(result.status_code, HTTPStatus.OK)
+    #     self.assertEqual(result.status_code, HTTPStatus.OK)
-        result_data = result.json()
+    #     result_data = result.json()
-        self.assertEqual(len(result_data["layout"]["coordinates"]), 15)
+    #     self.assertEqual(len(result_data["layout"]["coordinates"]), 15)
 
     def test_bad_filter(self):
-        endpoints = ["layout/obs", "annotations/obs", "annotations/var", "data/obs", "data/var"]
+        endpoints = ["annotations/obs", "annotations/var", "data/obs", "data/var"]
         for endpoint in endpoints:
             url = f"{URL_BASE}{endpoint}"
             result = self.session.put(url, json=BAD_FILTER)