bug(comp): several bug fixes...

* Channels could be out of order after compensating
* Allow comp matrix to have a subset of the file's columns in Compensation.apply
* Verify that a file's stored events were retrieved with the same kwargs provided to Compensation.apply

Additionally cleans up use of FCS files in tests.

Author: gegnew

cellengine/resources/compensation.py

@@ -100,36 +100,46 @@ def dataframe_as_html(self):
         """Return the compensation matrix dataframe as HTML."""
         return self.dataframe._repr_html_()
 
-    def apply(self, file: "FcsFile", inplace: bool = False, **kwargs):
-        """
-        Compensate an FcsFile's data.
+    def apply(self, file: FcsFile, inplace: bool = True, **kwargs):
+        """Compensate an FcsFile's data.
 
         Args:
-            file (FcsFile): The FCS file to compensate.
-            inplace (bool): Compensate the file's data in-place.
+            file (FcsFile): The FcsFile to compensate.
+            inplace (bool): If True, modify the `FcsFile.events` with the result.
+                If False, return the compensated events.
             kwargs (Dict):
                 All arguments accepted by `FcsFile.get_events` are accepted here.
+                If the file's events have already been retrieved with the same
+                kwargs provided here, those stored events will be used.
+                Otherwise, the file's events will be retrieved from CellEngine.
         Returns:
             DataFrame: if ``inplace=True``, updates `FcsFile.events` for
                 the target FcsFile
         """
-        data = file.get_events(**kwargs, inplace=True, destination=None)
-
-        # spill -> comp by inverting
-        inverted = numpy.linalg.inv(self.dataframe)
+        if kwargs.items() == file._events_kwargs.items():
+            data = file.events
+        else:
+            data = file.get_events(inplace=inplace, destination=None, **kwargs)
 
         # Calculate matrix product for channels matching between file and comp
-        if data and data[self.channels]:
-            comped = data[self.channels]
-            comped = comped.dot(inverted)  # type: ignore
-            comped.columns = self.channels
-            data.update(comped)
+        cols = data.columns
+        ix = list(
+            filter(
+                None,
+                [channel if channel in cols else None for channel in self.channels],
+            )
+        )
+        if any(ix):
+            copy = data.copy()
+            comped = copy[ix]
+            comped = comped.dot(numpy.linalg.inv(self.dataframe))  # type: ignore
+            comped.columns = ix
+            copy.update(comped.astype(comped.dtypes[0]))
         else:
             raise IndexError(
                 "No channels from this file match those in the compensation."
             )
 
         if inplace:
-            file._events = data
-        else:
-            return data
+            file._events = copy
+        return copy

cellengine/resources/fcs_file.py

@@ -1,10 +1,9 @@
 from __future__ import annotations
+from cellengine.utils.parse_fcs_file import parse_fcs_file
 from cellengine.utils.dataclass_mixin import DataClassMixin, ReadOnly
 from dataclasses import dataclass, field
 from dataclasses_json import config
 from typing import Any, Dict, List, Optional, Union
-from fcsparser.api import FCSParser
-import pandas
 from pandas.core.frame import DataFrame
 
 import cellengine as ce
@@ -14,16 +13,18 @@
 
 @dataclass
 class FcsFile(DataClassMixin):
-    _annotations: str = field(metadata=config(field_name="annotations"))
     filename: str
-    is_control: str
     panel_name: str
     deleted: Optional[bool]
     panel: List[Dict[str, Any]]
     _id: str = field(
         metadata=config(field_name="_id"), default=ReadOnly()
     )  # type: ignore
     compensation: Optional[int] = None
+    is_control: Optional[bool] = None
+    _annotations: Optional[List[str]] = field(
+        metadata=config(field_name="annotations"), default=None
+    )
     crc32c: str = field(default=ReadOnly())  # type: ignore
     event_count: int = field(default=ReadOnly())  # type: ignore
     experiment_id: str = field(default=ReadOnly())  # type: ignore
@@ -39,6 +40,9 @@ class FcsFile(DataClassMixin):
     def __repr__(self):
         return f"FcsFile(_id='{self._id}', name='{self.name}')"
 
+    def __post_init__(self):
+        self._events_kwargs = {}
+
     @property
     def name(self):
         """Alias for `filename`."""
@@ -276,13 +280,13 @@ def get_events(
             If destination is a string, saves file to the destination and returns None.
         """
 
-        fresp = ce.APIClient().download_fcs_file(self.experiment_id, self._id, **kwargs)
-        if destination:
-            with open(destination, "wb") as file:
-                file.write(fresp)
-            return
-        parser = FCSParser.from_data(fresp)
-        events = pandas.DataFrame(parser.data, columns=parser.channel_names_n)
+        if inplace is True:
+            self._events_kwargs = kwargs
+
+        fresp = parse_fcs_file(
+            ce.APIClient().download_fcs_file(self.experiment_id, self._id, **kwargs),
+            destination=destination,
+        )
         if inplace:
-            self._events = events
-        return events
+            self.events = fresp
+        return fresp

cellengine/utils/parse_fcs_file.py

@@ -0,0 +1,12 @@
+from fcsparser.api import FCSParser
+from pandas.core.frame import DataFrame
+
+
+def parse_fcs_file(file: bytes, destination: str = None) -> DataFrame:
+    """Parse an FCS file to a Dataframe"""
+    if destination:
+        with open(destination, "wb") as loc:
+            loc.write(file)
+    else:
+        parser = FCSParser.from_data(file)
+        return DataFrame(parser.data, columns=parser.channel_names_n)

tests/conftest.py

@@ -38,6 +38,7 @@
     "fixtures.api-statistics",
     "fixtures.api-events",
     "fixtures.spillstring",
+    "fixtures.compensated_events",
 ]
 
 

tests/data/acea.json

@@ -0,0 +1 @@
+{"FSC-H":{"0":138757,"1":2265460,"2":1764200,"3":1141736,"4":1971316},"SSC-H":{"0":12964,"1":278241,"2":113642,"3":101310,"4":110507},"BL1-H":{"0":1564.7786865234,"1":-874.6184692383,"2":-713.8847045898,"3":1384.0349121094,"4":-4269.4731445312},"BL2-H":{"0":159.2403869629,"1":40.1197471619,"2":413.5137329102,"3":413.7367248535,"4":-494.6390991211},"BL4-H":{"0":358.1521911621,"1":767.3947143555,"2":94748.734375,"3":1309.9842529297,"4":-367.4680786133},"BL5-H":{"0":507.7327880859,"1":3116.5961914062,"2":22717.23046875,"3":22469.728515625,"4":45046.3359375},"RL1-H":{"0":258.6846923828,"1":630.6443481445,"2":-1535.5026855469,"3":-363.8207092285,"4":353.9656677246},"RL2-H":{"0":-46.4186859131,"1":917.6150512695,"2":71390.8515625,"3":23751.76953125,"4":126923.3671875},"VL1-H":{"0":308.7546081543,"1":21288.693359375,"2":17380.46484375,"3":6990.4873046875,"4":22613.78125},"VL2-H":{"0":4096.4223632812,"1":37982.15625,"2":14175.427734375,"3":11279.310546875,"4":12162.017578125},"VL3-H":{"0":-599.0392456055,"1":-167.088104248,"2":-659.5164794922,"3":-660.2655029297,"4":806.4384155273},"FSC-A":{"0":41612,"1":1125353,"2":690129,"3":457694,"4":761981},"SSC-A":{"0":3886,"1":111465,"2":38985,"3":34435,"4":38346},"BL1-A":{"0":2623.0192871094,"1":-5131.4921875,"2":2718.5319824219,"3":6351.1533203125,"4":-5807.2319335938},"BL2-A":{"0":252.2498016357,"1":126.8422698975,"2":1621.1063232422,"3":1894.4182128906,"4":-490.734161377},"BL4-A":{"0":476.7016296387,"1":687.8566894531,"2":150272.640625,"3":3924.1550292969,"4":-251.9639129639},"BL5-A":{"0":800.4254760742,"1":5294.1884765625,"2":37090.51953125,"3":35231.73828125,"4":70414.75},"RL1-A":{"0":586.5928955078,"1":2220.6091308594,"2":2739.4243164062,"3":-969.1270141602,"4":1738.6413574219},"RL2-A":{"0":-783.5219726562,"1":1646.0065917969,"2":125483.03125,"3":42060.16796875,"4":224123.875},"VL1-A":{"0":634.5665893555,"1":38083.5703125,"2":29632.32421875,"3":12616.3515625,"4":37282.18359375},"VL2-A":{"0":6043.6430664062,"1":60756.7109375,"2":24508.138671875,"3":22079.921875,"4":19058.962890625},"VL3-A":{"0":-1048.1103515625,"1":680.421875,"2":-2519.341796875,"3":-2800.7143554688,"4":1013.7341308594},"Width":{"0":45,"1":82,"2":75,"3":73,"4":75},"TIME":{"0":42574.0,"1":42839.00390625,"2":44266.0,"3":48457.0,"4":50867.0}}

tests/data/acea_compensated.json

@@ -0,0 +1,35 @@
+from cellengine.resources.fcs_file import FcsFile
+import json
+from cellengine.utils.parse_fcs_file import parse_fcs_file
+from cellengine.resources.compensation import Compensation
+import pytest
+from pandas import read_json, DataFrame
+
+
+@pytest.fixture(scope="function")
+def acea_events() -> DataFrame:
+    """Real events from 'Acea - Novocyte.fcs'"""
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    file = parse_fcs_file(events_body.read())
+    return file.astype(float)
+
+
+@pytest.fixture(scope="function")
+def acea_fcs_file(acea_events):
+    with open("tests/data/acea.json", "r") as f:
+        fcs_file = json.load(f)
+    file = FcsFile.from_dict(fcs_file)
+    file.events = acea_events
+    return file
+
+
+@pytest.fixture(scope="function")
+def acea_compensation(acea_fcs_file):
+    return Compensation.from_spill_string(acea_fcs_file.spill_string)
+
+
+@pytest.fixture(scope="function")
+def acea_events_compensated(acea_events):
+    """File-internal compensated events from 'Acea - Novocyte.fcs'"""
+    file = read_json("tests/data/acea_compensated.json")
+    return file.astype(float)

tests/fixtures/compensated_events.py

@@ -1,16 +1,26 @@
+from cellengine.utils.parse_fcs_file import parse_fcs_file
+from cellengine.resources.fcs_file import FcsFile
 import json
-from numpy import array
 import pytest
 import responses
 from pandas import DataFrame
+from pandas.testing import assert_frame_equal
+from numpy import identity
 from cellengine.resources.compensation import Compensation
 
 
 EXP_ID = "5d38a6f79fae87499999a74b"
 
 
-@pytest.fixture(scope="module")
-def compensation(ENDPOINT_BASE, client, compensations):
+@pytest.fixture(scope="function")
+def fcs_file(ENDPOINT_BASE, client, fcs_files):
+    file = fcs_files[0]
+    file.update({"experimentId": EXP_ID})
+    return FcsFile.from_dict(file)
+
+
+@pytest.fixture(scope="function")
+def compensation(ENDPOINT_BASE, client, fcs_file, compensations):
     comp = compensations[0]
     comp.update({"experimentId": EXP_ID})
     return Compensation.from_dict(comp)
@@ -98,3 +108,70 @@ def test_create_from_spill_string(spillstring):
         "Qdot655-A",
         "Qdot705-A",
     ]
+
+
+@responses.activate
+def test_apply_comp_errors_for_nonmatching_channels(
+    client, ENDPOINT_BASE, compensation, fcs_file
+):
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    responses.add(
+        responses.GET,
+        f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{fcs_file._id}.fcs",
+        body=events_body,
+    )
+    events = parse_fcs_file(client.download_fcs_file(EXP_ID, fcs_file._id))
+    fcs_file.events = events
+
+    with pytest.raises(IndexError):
+        compensation.apply(fcs_file)
+
+
+@responses.activate
+def test_apply_compensation_to_fcs_file_with_matching_kwargs(
+    client, ENDPOINT_BASE, compensation, fcs_file
+):
+    # Given: a Compensation with channels as a subset of the FcsFile events
+    responses.add(
+        responses.GET,
+        f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{fcs_file._id}.fcs",
+        body=open("tests/data/Acea - Novocyte.fcs", "rb"),
+    )
+    events = fcs_file.get_events(inplace=True, testKwarg="foo")
+    assert fcs_file._events_kwargs == {"testKwarg": "foo"}
+
+    ix = list(events.columns)
+    compensation.dataframe = DataFrame(identity(24), index=ix, columns=ix)
+    compensation.channels = ix
+
+    # When: a Compensation is applied
+    results = compensation.apply(fcs_file, testKwarg="foo")
+
+    # Then: events should be compensated
+    assert all(results == events)
+    assert (
+        responses.assert_call_count(
+            f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{fcs_file._id}.fcs?testKwarg=foo",
+            1,
+        )
+        is True
+    )
+
+
+@responses.activate
+def test_apply_comp_compensates_values(
+    acea_events_compensated, acea_fcs_file, acea_compensation
+):
+    """This test compares results from a file-internal compensation conducted
+    by the Python toolkit to one conducted by CellEngine. See
+    tests/fixtures/compensated_events.py for details on the fixtures used
+    here."""
+    # Given:
+    # - a file-internal compensation (see tests/fixtures/compensated_events.py)
+    # - an FcsFile with uncompensated events
+
+    # When: the Compensation is applied to a file
+    results = acea_compensation.apply(acea_fcs_file, inplace=False)
+
+    # Then: events should be compensated correctly
+    assert_frame_equal(results.head(5), acea_events_compensated.head(5))

tests/unit/resources/test_compensation.py

@@ -1,5 +1,7 @@
 import os
 import json
+from fcsparser.api import FCSParser
+from pandas.core.frame import DataFrame
 import responses
 from io import BufferedReader, BytesIO
 
@@ -77,11 +79,55 @@ def test_gets_file_internal_compensation(ENDPOINT_BASE, client, fcs_files, spill
     assert type(comp) == Compensation
 
 
+def test_parse_fcs_file():
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    parser = FCSParser.from_data(events_body.read())
+
+    events = DataFrame(parser.data, columns=parser.channel_names_n)
+    assert type(events) is DataFrame
+    assert tuple(events.columns) == parser.channel_names_n
+
+
+@responses.activate
+def test_parses_fcs_file_events(ENDPOINT_BASE, client, fcs_files):
+    file_data = fcs_files[0]
+    file = FcsFile.from_dict(file_data)
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    responses.add(
+        responses.GET,
+        f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{file._id}.fcs",
+        body=events_body,
+    )
+
+    # When:
+    data = file.get_events()
+
+    # Then:
+    type(data) is DataFrame
+
+
+@responses.activate
+def test_parses_fcs_file_events_inplace(ENDPOINT_BASE, client, fcs_files):
+    file_data = fcs_files[0]
+    file = FcsFile.from_dict(file_data)
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    responses.add(
+        responses.GET,
+        f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{file._id}.fcs",
+        body=events_body,
+    )
+
+    # When:
+    file.get_events(inplace=True)
+
+    # Then:
+    type(file.events) is DataFrame
+
+
 @responses.activate
 def test_save_events_to_file(ENDPOINT_BASE, client, fcs_files):
     file_data = fcs_files[0]
     file = FcsFile.from_dict(file_data)
-    events_body = open("tests/data/Acea - Novocyte.fcs")
     responses.add(
         responses.GET,
         f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{file._id}.fcs",
@@ -95,3 +141,21 @@ def test_save_events_to_file(ENDPOINT_BASE, client, fcs_files):
     with open("test.fcs", "r") as events:
         assert events.readline() == "test"
     os.remove("test.fcs")
+
+
+@responses.activate
+def test_get_events_save_kwargs(ENDPOINT_BASE, client, fcs_files):
+    file_data = fcs_files[0]
+    file = FcsFile.from_dict(file_data)
+    events_body = open("tests/data/Acea - Novocyte.fcs", "rb")
+    responses.add(
+        responses.GET,
+        f"{ENDPOINT_BASE}/experiments/{EXP_ID}/fcsfiles/{file._id}.fcs",
+        body=events_body,
+    )
+
+    # When:
+    file.get_events(inplace=True, compensatedQ=False, seed=10)
+
+    # Then:
+    assert file._events_kwargs == {"compensatedQ": False, "seed": 10}