From 210787cb5ddafb655da166810413fc74ebe37c17 Mon Sep 17 00:00:00 2001 From: Charlotte Weaver Date: Thu, 11 Oct 2018 15:23:31 -0700 Subject: [PATCH] Filter in engine (#307) * Add empty filter case * Filtering dataframes moved to engine instead of rest * minor changes from PR review * Minor fixes from PR review Pass {} instead of none if no filter chain exceptions typos --- server/app/driver/driver.py | 22 +++--- server/app/rest_api/rest.py | 94 +++++++++++------------ server/app/scanpy_engine/scanpy_engine.py | 56 ++++++++++---- server/app/util/utils.py | 12 +++ server/test/test_scanpy_engine.py | 36 ++++----- 5 files changed, 127 insertions(+), 93 deletions(-) diff --git a/server/app/driver/driver.py b/server/app/driver/driver.py index 94e1f89b..859e4cf5 100644 --- a/server/app/driver/driver.py +++ b/server/app/driver/driver.py @@ -57,7 +57,7 @@ class CXGDriver(metaclass=ABCMeta): Filter cells from data and return a subset of the data. They can operate on both obs and var dimension with indexing and filtering by annotation value. Filters are combined with the and operator. See REST specs for info on filter format: - https://docs.google.com/document/d/1Fxjp1SKtCk7l8QP9-7KAjGXL0eldi_qEnNT0NmlGzXI/edit#heading=h.8qc9q57amldx + https://github.com/chanzuckerberg/cellxgene/blob/master/docs/REST_API.md :param filter: dictionary with filter params :return: View into scanpy object with cells/genes filtered @@ -65,10 +65,10 @@ class CXGDriver(metaclass=ABCMeta): pass @abstractmethod - def annotation(self, df, axis, fields=None): + def annotation(self, filter, axis, fields=None): """ Gets annotation value for each observation - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params :param axis: string obs or var :param fields: list of keys for annotation to return, returns all annotation values if not set. :return: dict: names - list of fields in order, data - list of lists or metadata @@ -77,10 +77,10 @@ class CXGDriver(metaclass=ABCMeta): pass @abstractmethod - def data_frame(self, df, axis): + def data_frame(self, filter, axis): """ Retrieves data for each variable for observations in data frame - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params :param axis: string obs or var :return: { "var": list of variable ids, @@ -90,23 +90,25 @@ class CXGDriver(metaclass=ABCMeta): pass @abstractmethod - def diffexp(self, df1, df2, top_n): + def diffexp(self, filter1, filter2, top_n=None, interactive_limit=None): """ Computes the top differentially expressed variables between two observation sets. If dataframes contain a subset of variables, then statistics for all variables will be returned, otherwise only the top N vars will be returned. - :param df1: from filter_cells, dataframe containing first set of observations - :param df2: from filter_cells, dataframe containing second set of observations + :param filter1: filter: dictionary with filter params for first set of observations + :param filter2: filter: dictionary with filter params for second set of observations :param top_n: Limit results to top N (Top var mode only) + :param interactive_limit: -- don't compute if total # genes in dataframes are larger than this :return: top genes, stats and expression values for variables """ pass @abstractmethod - def layout(self, df): + def layout(self, filter, interactive_limit=None): """ Computes a n-d layout for cells through dimensionality reduction. - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params + :param interactive_limit: -- don't compute if total # genes in dataframes are larger than this :return: [cellid, x, y, ...] """ pass diff --git a/server/app/rest_api/rest.py b/server/app/rest_api/rest.py index dd10e2d2..f1316c7b 100644 --- a/server/app/rest_api/rest.py +++ b/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 MimeTypeError, get_mime_type +from server.app.util.utils import FilterError, InteractiveError, MimeTypeError, get_mime_type """ Sort order for routes @@ -153,7 +153,7 @@ class AnnotationsObsAPI(Resource): def get(self): fields = request.args.getlist("annotation-name", None) try: - annotation_response = current_app.data.annotation(current_app.data.data, "obs", fields) + 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) @@ -202,13 +202,11 @@ class AnnotationsObsAPI(Resource): def put(self): fields = request.args.getlist("annotation-name", None) try: - df = current_app.data.filter_dataframe(request.get_json()["filter"], include_uns=False) - except KeyError: - return make_response("Malformed filter", HTTPStatus.BAD_REQUEST) - try: - annotation_response = current_app.data.annotation(df, "obs", fields) + annotation_response = current_app.data.annotation(request.get_json()["filter"], "obs", fields) except KeyError: 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) @@ -248,7 +246,7 @@ class AnnotationsVarAPI(Resource): def get(self): fields = request.args.getlist("annotation-name", None) try: - annotation_response = current_app.data.annotation(current_app.data.data, "var", fields) + 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) @@ -295,13 +293,11 @@ class AnnotationsVarAPI(Resource): def put(self): fields = request.args.getlist("annotation-name", None) try: - df = current_app.data.filter_dataframe(request.get_json()["filter"], include_uns=False) - except KeyError: - return make_response("Malformed filter", HTTPStatus.BAD_REQUEST) - try: - annotation_response = current_app.data.annotation(df, "var", fields) + annotation_response = current_app.data.annotation(request.get_json()["filter"], "var", fields) except KeyError: return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST) + except FilterError as e: + return make_response("Malformed filter", HTTPStatus.BAD_REQUEST) return make_response(jsonify(annotation_response), HTTPStatus.OK) @@ -352,10 +348,6 @@ class DataObsAPI(Resource): filter_ = parse_filter(ImmutableMultiDict(args), current_app.data.schema['annotations']) except QueryStringError as e: return make_response(e.message, HTTPStatus.BAD_REQUEST) - try: - df = current_app.data.filter_dataframe(filter_, include_uns=False) - except KeyError: - return make_response("malformed filter", HTTPStatus.BAD_REQUEST) # TODO support CSV try: # TODO store mime_type when more than one is supported @@ -363,7 +355,10 @@ class DataObsAPI(Resource): header=request.accept_mimetypes) except MimeTypeError as e: return make_response(e.message, HTTPStatus.NOT_ACCEPTABLE) - return make_response((jsonify(current_app.data.data_frame(df, axis=Axis.OBS))), HTTPStatus.OK) + try: + 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) @swagger.doc({ "summary": "Get data (expression values) from the dataframe.", @@ -399,15 +394,15 @@ class DataObsAPI(Resource): def put(self): if not request.accept_mimetypes.best_match(["application/json", "text/csv"]): return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE) - try: - df = current_app.data.filter_dataframe(request.get_json()["filter"], include_uns=False) - except KeyError: - return make_response("malformed filter", HTTPStatus.BAD_REQUEST) try: get_mime_type(acceptable_types=["application/json"], header=request.accept_mimetypes) except MimeTypeError as e: return make_response(e.message, HTTPStatus.NOT_ACCEPTABLE) - return make_response((jsonify(current_app.data.data_frame(df, axis=Axis.OBS))), HTTPStatus.OK) + try: + return make_response((jsonify(current_app.data.data_frame(request.get_json()["filter"], axis=Axis.OBS))), + HTTPStatus.OK) + except FilterError as e: + return make_response(e.message, HTTPStatus.BAD_REQUEST) class DataVarAPI(Resource): @@ -457,16 +452,15 @@ class DataVarAPI(Resource): filter_ = parse_filter(ImmutableMultiDict(args), current_app.data.schema['annotations']) except QueryStringError as e: return make_response(e.message, HTTPStatus.BAD_REQUEST) - try: - df = current_app.data.filter_dataframe(filter_, include_uns=False) - except KeyError: - return make_response("malformed filter", HTTPStatus.BAD_REQUEST) try: get_mime_type(acceptable_types=["application/json"], query_param=accept_type, header=request.accept_mimetypes) except MimeTypeError as e: return make_response(e.message, HTTPStatus.NOT_ACCEPTABLE) - return make_response((jsonify(current_app.data.data_frame(df, axis=Axis.VAR))), HTTPStatus.OK) + try: + 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) @swagger.doc({ "summary": "Get data (expression values) from the dataframe.", @@ -502,16 +496,16 @@ class DataVarAPI(Resource): def put(self): if not request.accept_mimetypes.best_match(["application/json", "text/csv"]): return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE) - try: - df = current_app.data.filter_dataframe(request.get_json()["filter"], include_uns=False) - except KeyError: - return make_response("malformed filter", HTTPStatus.BAD_REQUEST) # TODO support CSV try: get_mime_type(acceptable_types=["application/json"], header=request.accept_mimetypes) except MimeTypeError as e: return make_response(e.message, HTTPStatus.NOT_ACCEPTABLE) - return make_response((jsonify(current_app.data.data_frame(df, axis=Axis.VAR))), HTTPStatus.OK) + try: + return make_response((jsonify(current_app.data.data_frame(request.get_json()["filter"], axis=Axis.VAR))), + HTTPStatus.OK) + except FilterError as e: + return make_response(e.message, HTTPStatus.BAD_REQUEST) class DiffExpObsAPI(Resource): @@ -605,18 +599,15 @@ class DiffExpObsAPI(Resource): if "varFilter" in args: set1_filter[Axis.VAR] = args["varFilter"]["filter"][Axis.VAR] set2_filter[Axis.VAR] = args["varFilter"]["filter"][Axis.VAR] - df1 = current_app.data.filter_dataframe(set1_filter, include_uns=False) - # TODO inverse - df2 = current_app.data.filter_dataframe(set2_filter, include_uns=False) - # exceeds size limit - if df1.shape[0] + df2.shape[0] > current_app.data.features["diffexp"]["interactiveLimit"]: - return make_response("Non-interactive request", HTTPStatus.FORBIDDEN) # mode count = args.get("count", None) try: - diffexp = current_app.data.diffexp(df1, df2, count) - except ValueError as ve: - return make_response(ve.message, HTTPStatus.BAD_REQUEST) + diffexp = current_app.data.diffexp(set1_filter, set2_filter, count, + current_app.data.features["diffexp"]["interactiveLimit"]) + except (ValueError, FilterError) as e: + 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) @@ -643,7 +634,7 @@ class LayoutObsAPI(Resource): } }) def get(self): - return make_response((jsonify({"layout": current_app.data.layout(current_app.data.data)})), HTTPStatus.OK) + return make_response((jsonify({"layout": current_app.data.layout({})})), HTTPStatus.OK) @swagger.doc({ "summary": "Observation layout for filtered subset.", @@ -681,12 +672,19 @@ class LayoutObsAPI(Resource): }) def post(self): try: - df = current_app.data.filter_dataframe(request.get_json()["filter"]) - except KeyError: - return make_response("Malformed filter", HTTPStatus.BAD_REQUEST) - if len(df.obs.index) > current_app.data.features["layout"]["obs"]["interactiveLimit"]: + return make_response( + jsonify({ + "layout": current_app.data.layout( + request.get_json()["filter"], + interactive_limit=current_app.data.features["layout"]["obs"]["interactiveLimit"] + ) + }), + HTTPStatus.OK + ) + except FilterError as e: + return make_response(e.message, HTTPStatus.BAD_REQUEST) + except InteractiveError: return make_response("Non-interactive request", HTTPStatus.FORBIDDEN) - return make_response((jsonify({"layout": current_app.data.layout(df)})), HTTPStatus.OK) def get_api_resources(): diff --git a/server/app/scanpy_engine/scanpy_engine.py b/server/app/scanpy_engine/scanpy_engine.py index 0340b3ce..c58cbd0e 100644 --- a/server/app/scanpy_engine/scanpy_engine.py +++ b/server/app/scanpy_engine/scanpy_engine.py @@ -10,6 +10,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 """ Sort order for methods @@ -30,7 +31,7 @@ class ScanpyEngine(CXGDriver): self.cell_count = self.data.shape[0] self.gene_count = self.data.shape[1] self._create_schema() - self.layout(self.data) + self.layout(None) def _create_schema(self): self.schema = { @@ -122,8 +123,7 @@ class ScanpyEngine(CXGDriver): def genes(self): return self.data.var.index.tolist() - # Can't seem to cache a view of a dataframe, need to investigate why - def filter_dataframe(self, filter, include_uns=True): + def filter_dataframe(self, filter, include_uns=False): """ Filter cells from data and return a subset of the data. They can operate on both obs and var dimension with indexing and filtering by annotation value. Filters are combined with the and operator. @@ -131,10 +131,12 @@ class ScanpyEngine(CXGDriver): # TODO update this link to swagger when it's done https://docs.google.com/document/d/1Fxjp1SKtCk7l8QP9-7KAjGXL0eldi_qEnNT0NmlGzXI/edit#heading=h.8qc9q57amldx - :param filter: dictionary with filter parames + :param filter: dictionary with filter params :param include_uns: bool, include unstructured annotations :return: View into scanpy object with cells/genes filtered """ + if not filter: + return self.data cells_idx = np.ones((self.cell_count,), dtype=bool) genes_idx = np.ones((self.gene_count,), dtype=bool) if Axis.OBS in filter: @@ -200,15 +202,19 @@ class ScanpyEngine(CXGDriver): return index # @cache.memoize() - def annotation(self, df, axis, fields=None): + def annotation(self, filter, axis, fields=None): """ Gets annotation value for each observation - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params :param axis: string obs or var :param fields: list of keys for annotation to return, returns all annotation values if not set. :return: dict: names - list of fields in order, data - list of lists or metadata [observation ids, val1, val2...] """ + try: + df = self.filter_dataframe(filter) + except KeyError as e: + raise FilterError(f"Error parsing filter: {e}") from e df_axis = getattr(df, axis) if not fields: fields = df_axis.columns.tolist() @@ -219,16 +225,20 @@ class ScanpyEngine(CXGDriver): } # @cache.memoize() - def data_frame(self, df, axis): + def data_frame(self, filter, axis): """ Retrieves data for each variable for observations in data frame - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params :param axis: string obs or var :return: { "var": list of variable ids, "obs": [cellid, var1 expression, var2 expression, ...], } """ + try: + df = self.filter_dataframe(filter) + except KeyError as e: + raise FilterError(f"Error parsing filter: {e}") from e var_idx = df.var.index.tolist() obs_idx = df.obs.index.tolist() values = df.X @@ -252,20 +262,31 @@ class ScanpyEngine(CXGDriver): return result # @cache.memoize() - def diffexp(self, df1, df2, top_n=None): + def diffexp(self, filter1, filter2, top_n=None, interactive_limit=None): """ Computes the top differentially expressed variables between two observation sets. If dataframes contain a subset of variables, then statistics for all variables will be returned, otherwise only the top N vars will be returned. - :param df1: from filter_cells, dataframe containing first set of observations - :param df2: from filter_cells, dataframe containing second set of observations + :param filter1: filter: dictionary with filter params for first set of observations + :param filter2: filter: dictionary with filter params for second set of observations :param top_n: Limit results to top N (Top var mode only) + :param interactive_limit: -- don't compute if total # genes in dataframes are larger than this :return: top genes, stats and expression values for variables """ + try: + df1 = self.filter_dataframe(filter1) + except KeyError as e: + raise FilterError(f"Error parsing filter for set 1: {e}") from e + # TODO df2 should be inverse if not filter2 provided + try: + df2 = self.filter_dataframe(filter2) + except KeyError as e: + raise FilterError(f"Error parsing filter for set 2: {e}") from e # If not the same genes, test is wrong! if np.any(df1.var.index != df2.var.index): raise ValueError("Variables ares not the same in set1 and set2") - + if interactive_limit and df1.shape[0] + df2.shape[0] > interactive_limit: + raise InteractiveError("Size of set 1 and 2 is too large for interactive computation") # If not all genes, they used a var filter if df1.var.shape[0] < self.gene_count: mode = DiffExpMode.VAR_FILTER @@ -301,12 +322,19 @@ class ScanpyEngine(CXGDriver): return sorted(result, key=lambda gene: gene[0]) # @cache.memoize() - def layout(self, df): + def layout(self, filter, interactive_limit=None): """ Computes a n-d layout for cells through dimensionality reduction. - :param df: from filter_cells, dataframe + :param filter: filter: dictionary with filter params + :param interactive_limit: -- don't compute if total # genes in dataframes are larger than this :return: [cellid, x, y, ...] """ + try: + df = self.filter_dataframe(filter, include_uns=True) + except KeyError as e: + raise FilterError(f"Error parsing filter: {e}") from e + if interactive_limit and len(df.obs.index) > interactive_limit: + raise InteractiveError("Size data is too large for interactive computation") # 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 diff --git a/server/app/util/utils.py b/server/app/util/utils.py index 7c4419ff..54661ac5 100644 --- a/server/app/util/utils.py +++ b/server/app/util/utils.py @@ -18,6 +18,18 @@ class MimeTypeError(Exception): self.message = message +class FilterError(Exception): + + def __init__(self, message): + self.message = message + + +class InteractiveError(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 diff --git a/server/test/test_scanpy_engine.py b/server/test/test_scanpy_engine.py index f0b15292..c803967f 100644 --- a/server/test/test_scanpy_engine.py +++ b/server/test/test_scanpy_engine.py @@ -120,7 +120,7 @@ class UtilTest(unittest.TestCase): self.assertEqual(self.data.features["layout"]["obs"], {'available': True, 'interactiveLimit': 15000}) def test_layout(self): - layout = self.data.layout(self.data.data) + layout = self.data.layout(None) self.assertEqual(layout["ndims"], 2) self.assertEqual(len(layout["coordinates"]), 2638) self.assertEqual(layout["coordinates"][0][0], 0) @@ -129,18 +129,18 @@ class UtilTest(unittest.TestCase): self.assertLessEqual(val[2], 1) def test_annotations(self): - annotations = self.data.annotation(self.data.data, "obs") + annotations = self.data.annotation(None, "obs") self.assertEqual(annotations["names"], ["n_genes", "percent_mito", "n_counts", "louvain", "name"]) self.assertEqual(len(annotations["data"]), 2638) - annotations = self.data.annotation(self.data.data, "var") + annotations = self.data.annotation(None, "var") self.assertEqual(annotations["names"], ["n_cells", "name"]) self.assertEqual(len(annotations["data"]), 1838) def test_annotation_fields(self): - annotations = self.data.annotation(self.data.data, "obs", ["n_genes", "n_counts"]) + annotations = self.data.annotation(None, "obs", ["n_genes", "n_counts"]) self.assertEqual(annotations["names"], ["n_genes", "n_counts"]) self.assertEqual(len(annotations["data"]), 2638) - annotations = self.data.annotation(self.data.data, "var", ["name"]) + annotations = self.data.annotation(None, "var", ["name"]) self.assertEqual(annotations["names"], ["name"]) self.assertEqual(len(annotations["data"]), 1838) @@ -159,11 +159,10 @@ class UtilTest(unittest.TestCase): } } } - data = self.data.filter_dataframe(filter_["filter"]) - annotations = self.data.annotation(data, "obs") + annotations = self.data.annotation(filter_["filter"], "obs") self.assertEqual(annotations["names"], ["n_genes", "percent_mito", "n_counts", "louvain", "name"]) self.assertEqual(len(annotations["data"]), 497) - annotations = self.data.annotation(data, "var") + annotations = self.data.annotation(filter_["filter"], "var") self.assertEqual(annotations["names"], ["n_cells", "name"]) self.assertEqual(len(annotations["data"]), 2) @@ -177,8 +176,7 @@ class UtilTest(unittest.TestCase): } } } - data = self.data.filter_dataframe(filter_["filter"]) - layout = self.data.layout(data) + layout = self.data.layout(filter_["filter"]) self.assertEqual(len(layout["coordinates"]), 497) def test_diffexp(self): @@ -189,7 +187,6 @@ class UtilTest(unittest.TestCase): } } } - df1 = self.data.filter_dataframe(f1["filter"]) f2 = { "filter": { "obs": { @@ -197,19 +194,18 @@ class UtilTest(unittest.TestCase): } } } - df2 = self.data.filter_dataframe(f2["filter"]) - result = self.data.diffexp(df1, df2) + result = self.data.diffexp(f1["filter"], f2["filter"]) self.assertEqual(len(result), 10) var_idx = [i[0] for i in result] self.assertEqual(var_idx, sorted(var_idx)) - result = self.data.diffexp(df1, df2, 20) + result = self.data.diffexp(f1["filter"], f2["filter"], 20) self.assertEqual(len(result), 20) def test_data_frame(self): - data_frame_obs = self.data.data_frame(self.data.data, "obs") + data_frame_obs = self.data.data_frame(None, "obs") self.assertEqual(len(data_frame_obs["var"]), 1838) self.assertEqual(len(data_frame_obs["obs"]), 2638) - data_frame_var = self.data.data_frame(self.data.data, "var") + data_frame_var = self.data.data_frame(None, "var") self.assertEqual(len(data_frame_var["var"]), 1838) self.assertEqual(len(data_frame_var["obs"]), 2638) @@ -223,13 +219,12 @@ class UtilTest(unittest.TestCase): } } } - data = self.data.filter_dataframe(filter_["filter"]) - data_frame_obs = self.data.data_frame(data, "obs") + data_frame_obs = self.data.data_frame(filter_["filter"], "obs") self.assertEqual(len(data_frame_obs["var"]), 1838) self.assertEqual(len(data_frame_obs["obs"]), 497) self.assertEqual(type(data_frame_obs["obs"][0]), list) self.assertEqual(type(data_frame_obs["var"][0]), int) - data_frame_var = self.data.data_frame(data, "var") + data_frame_var = self.data.data_frame(filter_["filter"], "var") self.assertEqual(len(data_frame_var["var"]), 1838) self.assertEqual(len(data_frame_var["obs"]), 497) self.assertEqual(type(data_frame_var["var"][0]), list) @@ -246,8 +241,7 @@ class UtilTest(unittest.TestCase): } } } - data = self.data.filter_dataframe(filter_["filter"], include_uns=False) - data_frame_var = self.data.data_frame(data, axis) + data_frame_var = self.data.data_frame(filter_["filter"], axis) if axis == "obs": self.assertEqual(type(data_frame_var["var"][0]), int) self.assertEqual(type(data_frame_var["obs"][0]), list)