From f91a3060840990f0209992bfe6c4fc3d616101fc Mon Sep 17 00:00:00 2001 From: Zach Bjornson Date: Mon, 13 Dec 2021 13:30:39 -0800 Subject: [PATCH 1/2] bug(*): fix Boolean query params Affects plots and anything else where CellEngine's API is case-sensitive (might only be plots). We'll make the API case-insensitive at some point soon as well. Fixes #124 --- cellengine/utils/api_client/APIClient.py | 2 +- cellengine/utils/api_client/BaseAPIClient.py | 13 +++++++++---- tests/unit/resources/test_plots.py | 6 +++++- tests/unit/utils/test_lru_cache.py | 3 ++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cellengine/utils/api_client/APIClient.py b/cellengine/utils/api_client/APIClient.py index 9a98627d..ded56d8a 100644 --- a/cellengine/utils/api_client/APIClient.py +++ b/cellengine/utils/api_client/APIClient.py @@ -97,7 +97,7 @@ def _get_id_by_name(self, name, resource_type, experiment_id): raise RuntimeError(str(e).format(name)) def _lookup_by_name(self, path, query, name): - params = f'query=eq({query},"{name}")&limit=2' + params = {"query": f'eq({query},"{name}")', "limit": 2} return self._get(f"{self.base_url}/{path}", params=params) def _handle_response(self, response): diff --git a/cellengine/utils/api_client/BaseAPIClient.py b/cellengine/utils/api_client/BaseAPIClient.py index bc991171..03cb7a77 100644 --- a/cellengine/utils/api_client/BaseAPIClient.py +++ b/cellengine/utils/api_client/BaseAPIClient.py @@ -11,6 +11,11 @@ from cellengine.utils.singleton import AbstractSingleton +def prepare_params(params: Dict) -> Dict: + """Converts Booleans to lower-case strings. (Requests yields upper-case.)""" + return {k: str(v).lower() if type(v) == bool else v for k, v in params.items()} + + class BaseAPIClient(metaclass=AbstractSingleton): @property @abstractmethod @@ -67,7 +72,7 @@ def _get(self, url, params: Dict = None, headers: Dict = None, raw=False) -> Any response = self.requests_session.get( url, headers=self._make_headers(headers), - params=params if params else {}, + params=prepare_params(params if params else {}), ) except Exception as error: raise error @@ -87,7 +92,7 @@ def _post( url, json=json, headers=self._make_headers(headers), - params=params if params else {}, + params=prepare_params(params if params else {}), files=files, data=data, ) @@ -106,7 +111,7 @@ def _patch( url, json=json, headers=self._make_headers(headers), - params=params if params else {}, + params=prepare_params(params if params else {}), files=files, ) return self._parse_response(response, raw=raw) @@ -115,7 +120,7 @@ def _delete(self, url, params: dict = None, headers: dict = None): response = self.requests_session.delete( url, headers=self._make_headers(headers), - params=params if params else {}, + params=prepare_params(params if params else {}), ) try: if response.ok: diff --git a/tests/unit/resources/test_plots.py b/tests/unit/resources/test_plots.py index 529f9f8c..71d378d0 100644 --- a/tests/unit/resources/test_plots.py +++ b/tests/unit/resources/test_plots.py @@ -101,7 +101,11 @@ def test_should_get_plot_for_each_query_parameter( item = ("color", "%23ff0000") assert item[0] in responses.calls[i].request.url - assert str(item[1]) in responses.calls[i].request.url + assert ( + str(item[1]).lower() + if type(item[1]) is bool + else str(item[1]) in responses.calls[i].request.url + ) plot_tester(plot) i += 1 diff --git a/tests/unit/utils/test_lru_cache.py b/tests/unit/utils/test_lru_cache.py index 4c0f2ccd..ea2c3366 100644 --- a/tests/unit/utils/test_lru_cache.py +++ b/tests/unit/utils/test_lru_cache.py @@ -52,7 +52,8 @@ def test_lru_cache_paths(ENDPOINT_BASE, client, experiments): client.get_experiment(name="test_experiment") assert ( responses.calls[0].request.url - == ENDPOINT_BASE + "/experiments?query=eq(name,%22test_experiment%22)&limit=2" + == ENDPOINT_BASE + + "/experiments?query=eq%28name%2C%22test_experiment%22%29&limit=2" ) responses.add(responses.GET, ENDPOINT_BASE + "/experiments", json=experiments[0]) From 65986d17867810e5738a9a3e2ec4b531ca826da2 Mon Sep 17 00:00:00 2001 From: Zach Bjornson Date: Mon, 13 Dec 2021 15:12:45 -0800 Subject: [PATCH 2/2] bug(gates): fix `create_population` argument handling Fixes #118 --- .../payloads/gate_utils/ellipse_gate.py | 4 --- .../payloads/gate_utils/polygon_gate.py | 3 -- .../payloads/gate_utils/quadrant_gate.py | 3 -- cellengine/payloads/gate_utils/range_gate.py | 3 -- .../payloads/gate_utils/rectangle_gate.py | 3 -- cellengine/payloads/gate_utils/split_gate.py | 3 -- cellengine/payloads/gate_utils/utils.py | 5 +--- cellengine/resources/experiment.py | 30 +++++++++++++++---- cellengine/resources/gate.py | 24 +++++++++++---- tests/unit/resources/test_gates.py | 25 ++++++++++++++++ 10 files changed, 68 insertions(+), 35 deletions(-) diff --git a/cellengine/payloads/gate_utils/ellipse_gate.py b/cellengine/payloads/gate_utils/ellipse_gate.py index 9c1e5bd0..3c6af1ee 100644 --- a/cellengine/payloads/gate_utils/ellipse_gate.py +++ b/cellengine/payloads/gate_utils/ellipse_gate.py @@ -22,7 +22,6 @@ def format_ellipse_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """Formats an ellipse gate for posting to the CellEngine API. @@ -60,8 +59,6 @@ def format_ellipse_gate( files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (optional, bool): Automatically create corresponding - population. Returns: EllipseGate: An EllipseGate object. @@ -101,5 +98,4 @@ def format_ellipse_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/polygon_gate.py b/cellengine/payloads/gate_utils/polygon_gate.py index e9330e83..da797b59 100644 --- a/cellengine/payloads/gate_utils/polygon_gate.py +++ b/cellengine/payloads/gate_utils/polygon_gate.py @@ -19,7 +19,6 @@ def format_polygon_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """Formats a polygon gate for posting to the CellEngine API. @@ -49,7 +48,6 @@ def format_polygon_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. Returns: A PolygonGate object. @@ -91,5 +89,4 @@ def format_polygon_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/quadrant_gate.py b/cellengine/payloads/gate_utils/quadrant_gate.py index d546ab0a..3f724fc6 100644 --- a/cellengine/payloads/gate_utils/quadrant_gate.py +++ b/cellengine/payloads/gate_utils/quadrant_gate.py @@ -24,7 +24,6 @@ def format_quadrant_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """Formats a quadrant gate for posting to the CellEngine API. @@ -64,7 +63,6 @@ def format_quadrant_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. Returns: A QuadrantGate object. @@ -132,5 +130,4 @@ def format_quadrant_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/range_gate.py b/cellengine/payloads/gate_utils/range_gate.py index a4e929c7..f7b2cc3b 100644 --- a/cellengine/payloads/gate_utils/range_gate.py +++ b/cellengine/payloads/gate_utils/range_gate.py @@ -20,7 +20,6 @@ def format_range_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """Formats a range gate for posting to the CellEngine API. @@ -52,7 +51,6 @@ def format_range_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. Returns: A RangeGate object. @@ -89,5 +87,4 @@ def format_range_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/rectangle_gate.py b/cellengine/payloads/gate_utils/rectangle_gate.py index 58166d8e..45cbfd94 100644 --- a/cellengine/payloads/gate_utils/rectangle_gate.py +++ b/cellengine/payloads/gate_utils/rectangle_gate.py @@ -22,7 +22,6 @@ def format_rectangle_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """Formats a rectangle gate for posting to the CellEngine API. @@ -57,7 +56,6 @@ def format_rectangle_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. Returns: A RectangleGate object. @@ -99,5 +97,4 @@ def format_rectangle_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/split_gate.py b/cellengine/payloads/gate_utils/split_gate.py index 94f254c1..34f02caf 100644 --- a/cellengine/payloads/gate_utils/split_gate.py +++ b/cellengine/payloads/gate_utils/split_gate.py @@ -20,7 +20,6 @@ def format_split_gate( tailored_per_file: bool = False, fcs_file_id: str = None, fcs_file: str = None, - create_population: bool = True, ): """ Formats a split gate for posting to the CellEngine API. @@ -58,7 +57,6 @@ def format_split_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. Returns: A SplitGate object. @@ -116,5 +114,4 @@ def format_split_gate( tailored_per_file=tailored_per_file, fcs_file_id=fcs_file_id, fcs_file=fcs_file, - create_population=create_population, ) diff --git a/cellengine/payloads/gate_utils/utils.py b/cellengine/payloads/gate_utils/utils.py index 9bca956d..b2f15b54 100644 --- a/cellengine/payloads/gate_utils/utils.py +++ b/cellengine/payloads/gate_utils/utils.py @@ -1,9 +1,7 @@ import cellengine as ce -def format_common_gate( - experiment_id, body, tailored_per_file, fcs_file_id, fcs_file, create_population -): +def format_common_gate(experiment_id, body, tailored_per_file, fcs_file_id, fcs_file): """ Args: experiment_id (str): The ID of the experiment to which to add the gate. @@ -30,7 +28,6 @@ def format_common_gate( the name, an error will be thrown. Looking up files by name is slower than using the ID, as this requires additional requests to the server. If specified, do not specify ``fcs_file_id``. - create_population (bool): Automatically create corresponding population. """ return parse_fcs_file_args( experiment_id, body, tailored_per_file, fcs_file_id, fcs_file diff --git a/cellengine/resources/experiment.py b/cellengine/resources/experiment.py index 4d200f13..2dc3acad 100644 --- a/cellengine/resources/experiment.py +++ b/cellengine/resources/experiment.py @@ -431,11 +431,14 @@ def create_rectangle_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_rectangle_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore @doc_inherit(format_polygon_gate) def create_polygon_gate( @@ -457,11 +460,14 @@ def create_polygon_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_polygon_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore @doc_inherit(format_ellipse_gate) def create_ellipse_gate( @@ -487,11 +493,14 @@ def create_ellipse_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_ellipse_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore @doc_inherit(format_range_gate) def create_range_gate( @@ -514,11 +523,14 @@ def create_range_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_range_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore @doc_inherit(format_split_gate) def create_split_gate( @@ -541,11 +553,14 @@ def create_split_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_split_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore @doc_inherit(format_quadrant_gate) def create_quadrant_gate( @@ -571,11 +586,14 @@ def create_quadrant_gate( f = inspect.currentframe() args, _, _, values = inspect.getargvalues(f) # type: ignore kwargs = {arg: values.get(arg, None) for arg in args} + del kwargs["create_population"] post_body = format_quadrant_gate( kwargs.pop("self")._id, # type: ignore **kwargs, ) - return ce.APIClient().post_gate(self._id, post_body) # type: ignore + return ce.APIClient().post_gate( + self._id, post_body, create_population + ) # type: ignore def create_population(self, population: Dict) -> Population: """Create a complex population diff --git a/cellengine/resources/gate.py b/cellengine/resources/gate.py index bcc18be5..4fa24441 100644 --- a/cellengine/resources/gate.py +++ b/cellengine/resources/gate.py @@ -203,7 +203,9 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> RectangleGate: - g = format_rectangle_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_rectangle_gate(**args) return cls(g) @@ -229,7 +231,9 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> PolygonGate: - g = format_polygon_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_polygon_gate(**args) return cls(g) @@ -259,7 +263,9 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> EllipseGate: - g = format_ellipse_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_ellipse_gate(**args) return cls(g) @@ -286,7 +292,9 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> RangeGate: - g = format_range_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_range_gate(**args) return cls(g) @@ -316,7 +324,9 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> QuadrantGate: - g = format_quadrant_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_quadrant_gate(**args) return cls(g) @@ -343,5 +353,7 @@ def create( fcs_file: str = None, create_population: bool = True, ) -> SplitGate: - g = format_split_gate(**get_args_as_kwargs(cls, locals())) + args = get_args_as_kwargs(cls, locals()) + del args["create_population"] + g = format_split_gate(**args) return cls(g) diff --git a/tests/unit/resources/test_gates.py b/tests/unit/resources/test_gates.py index b328983b..43690915 100644 --- a/tests/unit/resources/test_gates.py +++ b/tests/unit/resources/test_gates.py @@ -277,6 +277,31 @@ def test_create_rectangle_gate(ENDPOINT_BASE, client, experiment, rectangle_gate assert rectangle_gate.model.rectangle.y2 == 215000 +@responses.activate +def test_create_rectangle_gate_without_create_population( + ENDPOINT_BASE, client, experiment, rectangle_gate +): + """Regression test for #118.""" + responses.add( + responses.POST, + f"{ENDPOINT_BASE}/experiments/{EXP_ID}/gates", + status=201, + json=rectangle_gate, + ) + rectangle_gate = experiment.create_rectangle_gate( + x_channel="FSC-A", + y_channel="FSC-W", + name="my gate", + x1=60000, + x2=200000, + y1=75000, + y2=215000, + create_population=False, + ) + rectangle_gate.post() + assert "createPopulation=false" in responses.calls[0].request.url + + @responses.activate def test_create_ellipse_gate(ENDPOINT_BASE, client, experiment, ellipse_gate): responses.add(