feat: Add support for transfers from S3 to FcsFile.create

gegnew · zbjornson · commit 66c5a1cf50a2 · 2022-02-28T19:09:29.000-08:00
diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml
@@ -46,6 +46,9 @@ jobs:
           pip install pytest
           pip install pytest-cov
           python -m pytest --cov=./ --cov-report=xml
+        env:
+          S3_ACCESS_KEY: ${{ secrets.S3_ACCESS_KEY}}
+          S3_SECRET_KEY: ${{ secrets.S3_SECRET_KEY}}
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v1
         with:
diff --git a/cellengine/resources/fcs_file.py b/cellengine/resources/fcs_file.py
@@ -1,4 +1,5 @@
 from __future__ import annotations
+from cellengine.utils.helpers import is_valid_id
 from cellengine.utils.parse_fcs_file import parse_fcs_file
 from cellengine.utils.dataclass_mixin import DataClassMixin, ReadOnly
 from dataclasses import dataclass, field
@@ -102,25 +103,65 @@ def upload(cls, experiment_id: str, filepath: str) -> FcsFile:
     def create(
         cls,
         experiment_id: str,
-        fcs_files: List[str],
+        fcs_files: Union[str, List[str], Dict[str, str]],
         filename: str = None,
         add_file_number: bool = False,
         add_event_number: bool = False,
         pre_subsample_n: int = None,
         pre_subsample_p: float = None,
         seed: int = None,
     ) -> FcsFile:
-        """Creates an FCS file by copying, concatenating and/or subsampling
-        existing file(s) from this or other experiments.
-
-        This endpoint can be used to import files from other experiments.
+        """Creates an FCS file by copying, concatenating and/or
+        subsampling existing file(s) from this or other experiments, or by
+        importing from an S3-compatible service. This endpoint can be used to
+        import files from other experiments.
+
+        When concatenating and subsampling at the same time, subsampling is
+        applied to each file before concatenating.
+
+        If addFileNumber is true, a file number column (channel) will be added to the
+        output file indicating which file each event (cell) came from. The values in
+        this column have a uniform random spread (±0.25 of the integer value) to ease
+        visualization. While this column can be useful for analysis, it will cause the
+        experiment to have FCS files with different panels unless all FCS files that
+        have not been concatenated are deleted.
+
+        During concatenation, any FCS header parameters that do not match
+        between files will be removed, with some exceptions:
+
+            - $BTIM (clock time at beginning of acquisition) and $DATE will be
+            set to the earliest value among the input files.
+            - $ETIM (clock time at end of acquisition) will be set to the latest value
+            among the input files.
+            - $PnR (range for parameter n) will be set to
+            the highest value among the input files.
+
+        All channels present in the first FCS file in the fcsFiles parameter
+        must also be present in the other FCS files.
+
+        When importing from an S3-compatible service, be aware of the
+        following:
+
+            - Only a single file can be imported at a time.
+            - The host property must include the bucket and region as
+              applicable. For example, for AWS, this would look like
+              mybucket.s3.us-east-2.amazonaws.com.
+            - The path property must specify the full path to the object, e.g.
+              /Study001/Specimen01.fcs.
+            - Importing private S3 objects requires an accessKey and a
+              secretKey for a user with appropriate permissions. For AWS,
+              GetObject is required.
+            - Importing objects may incur fees from the S3 service provider.
 
         Args:
             experiment_id: ID of the experiment to which the file belongs
             fcs_files: ID of file or list of IDs of files or objects to process.
                 If more than one file is provided, they will be concatenated in
                 order. To import files from other experiments, pass a list of dicts
-                with _id and experimentId properties.
+                with _id and experimentId properties. To import a file from an
+                S3-compatible service, provide a Dict with keys "host" and
+                "path"; if the S3 object is private, additionally provide
+                "access_key" and "secret_key".
             filename (optional): Rename the uploaded file.
             add_file_number (optional): If
                 concatenating files, adds a file number channel to the
@@ -142,10 +183,17 @@ def create(
         """
 
         def _parse_fcs_file_args(args):
-            if type(args) is list:
+            if type(args) is list and all(is_valid_id(arg) for arg in args):
                 return args
-            else:
+            elif type(args) is dict:
+                if {"host", "path"} <= args.keys():
+                    return [args]
+                if {"_id", "experiment_id"} <= args.keys():
+                    return [args]
+            elif type(args) is str and is_valid_id(args):
                 return [args]
+            else:
+                raise ValueError("Invalid parameters for 'fcs_file'.")
 
         body = {"fcsFiles": _parse_fcs_file_args(fcs_files), "filename": filename}
         optional_params = {
@@ -179,7 +227,7 @@ def plot(
         population_id: str = None,
         **kwargs,
     ) -> Plot:
-        """Buid a plot for an FcsFile.
+        """Build a plot for an FcsFile.
 
         See [`Plot.get`][cellengine.resources.plot.Plot.get] for more information.
         """
diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py
@@ -1,3 +1,4 @@
+import os
 import pytest
 import pandas
 
@@ -18,7 +19,9 @@
 
 @pytest.fixture(scope="module")
 def client():
-    return cellengine.APIClient("gegnew", "testpass123")
+    username = os.environ.get("CELLENGINE_USERNAME", "gegnew")
+    password = os.environ.get("CELLENGINE_PASSWORD", "testpass1")
+    return cellengine.APIClient(username=username, password=password)
 
 
 @pytest.fixture(scope="module")
@@ -255,3 +258,20 @@ def test_experiment_populations(setup_experiment, client):
     # DELETE
     complex_pop.delete()
     assert "complex pop" not in [p.name for p in experiment.populations]
+
+
+def test_create_new_fcsfile_from_s3(setup_experiment, client):
+    experiment = client.get_experiment(name="new_experiment")
+    s3_dict = {
+        "host": "ce-test-s3-a.s3.us-east-2.amazonaws.com",
+        "path": "/Specimen_001_A6_A06.fcs",
+        "access_key": os.environ.get("S3_ACCESS_KEY"),
+        "secret_key": os.environ.get("S3_SECRET_KEY"),
+    }
+
+    file = FcsFile.create(
+        experiment._id,
+        s3_dict,
+        "new name",
+    )
+    assert file.size == 22625
diff --git a/tests/unit/resources/test_fcs_file_parse_args.py b/tests/unit/resources/test_fcs_file_parse_args.py
@@ -38,14 +38,37 @@ def test_should_create_fcs_file(ENDPOINT_BASE, client, fcs_files):
 
 
 params = [
+    # (request, expected response)
     (FCSFILE_ID, [FCSFILE_ID]),
     ([FCSFILE_ID], [FCSFILE_ID]),
     (
-        ["fcs_file_id_1", "fcs_file_id_2", "fcs_file_id_3"],
-        ["fcs_file_id_1", "fcs_file_id_2", "fcs_file_id_3"],
+        [
+            "5d64abe2ca9df61349ed8e7a",
+            "5d64abe2ca9df61349ed8e7b",
+            "5d64abe2ca9df61349ed8e7c",
+        ],
+        [
+            "5d64abe2ca9df61349ed8e7a",
+            "5d64abe2ca9df61349ed8e7b",
+            "5d64abe2ca9df61349ed8e7c",
+        ],
+    ),
+    (
+        {"experiment_id": EXP_ID, "_id": FCSFILE_ID},
+        [{"experimentId": EXP_ID, "_id": FCSFILE_ID}],
+    ),
+    (
+        {
+            "host": "ce-test-s3-a.s3.us-east-2.amazonaws.com",
+            "path": "/Specimen_001_A6_A06.fcs",
+        },
+        [
+            {
+                "host": "ce-test-s3-a.s3.us-east-2.amazonaws.com",
+                "path": "/Specimen_001_A6_A06.fcs",
+            }
+        ],
     ),
-    ({EXP_ID: FCSFILE_ID}, [{EXP_ID: FCSFILE_ID}]),
-    ([{EXP_ID: FCSFILE_ID}], [{EXP_ID: FCSFILE_ID}]),
 ]
 
 
diff --git a/tests/unit/resources/test_fcsfile.py b/tests/unit/resources/test_fcsfile.py
@@ -7,6 +7,7 @@
 
 from cellengine.resources.fcs_file import FcsFile
 from cellengine.resources.compensation import Compensation
+from cellengine.utils.helpers import to_camel_case
 
 
 EXP_ID = "5d38a6f79fae87499999a74b"
@@ -159,3 +160,52 @@ def test_get_events_save_kwargs(ENDPOINT_BASE, client, fcs_files):
 
     # Then:
     assert file._events_kwargs == {"compensatedQ": False, "seed": 10}
+
+
+@responses.activate
+def test_create_from_s3(ENDPOINT_BASE, client, fcs_files):
+    responses.add(
+        responses.POST,
+        ENDPOINT_BASE + f"/experiments/{EXP_ID}/fcsfiles",
+        json=fcs_files[1],
+    )
+
+    s3_dict = {
+        "host": "ce-test-s3-a.s3.us-east-2.amazonaws.com",
+        "path": "/Specimen_001_A6_A06.fcs",
+        "access_key": os.environ.get("S3_ACCESS_KEY"),
+        "secret_key": os.environ.get("S3_SECRET_KEY"),
+    }
+
+    FcsFile.create(
+        EXP_ID,
+        s3_dict,
+        "new name",
+    )
+    payload = json.loads(responses.calls[0].request.body)["fcsFiles"][0]  # type: ignore
+
+    assert {"host", "path", "accessKey", "secretKey"} <= payload.keys()
+    assert payload == {to_camel_case(k): v for k, v in s3_dict.items()}
+
+
+@responses.activate
+def test_create_from_another_experiment(ENDPOINT_BASE, client, fcs_files):
+    responses.add(
+        responses.POST,
+        ENDPOINT_BASE + f"/experiments/{EXP_ID}/fcsfiles",
+        json=fcs_files[1],
+    )
+
+    file_dict = {
+        "_id": fcs_files[1]["_id"],
+        "experiment_id": EXP_ID,
+    }
+
+    FcsFile.create(
+        EXP_ID,
+        file_dict,
+        "new name",
+    )
+    payload = json.loads(responses.calls[0].request.body)  # type: ignore
+
+    assert {"_id", "experimentId"} <= payload["fcsFiles"][0].keys()
diff --git a/tests/unit/utils/test_helpers.py b/tests/unit/utils/test_helpers.py
@@ -9,7 +9,7 @@ def test_alter_keys_converts_dict_to_camel_case():
         "fcsFile": "baz",
     }
     r = alter_keys(d, to_camel_case)
-    assert {"_id", "experimentId", "fcsFile"} <= r.keys()
+    assert {"_id", "experimentId", "fcsFile"} == r.keys()
 
 
 def test_alter_keys_converts_nested_dict_to_camel_case():
@@ -23,8 +23,8 @@ def test_alter_keys_converts_nested_dict_to_camel_case():
         },
     }
     r = alter_keys(d, to_camel_case)
-    assert {"_id", "experimentId", "fcsFile"} <= r.keys()
-    assert {"_id", "experimentId", "fcsFile"} <= r["fcsFile"].keys()
+    assert {"_id", "experimentId", "fcsFile"} == r.keys()
+    assert {"_id", "experimentId", "fcsFile"} == r["fcsFile"].keys()
 
 
 def test_alter_keys_converts_list_of_dicts_to_camel_case():
@@ -41,7 +41,7 @@ def test_alter_keys_converts_list_of_dicts_to_camel_case():
         },
     ]
     r = alter_keys(d, to_camel_case)
-    assert {"_id", "experimentId", "fcsFile"} <= r[0].keys()
+    assert {"_id", "experimentId", "fcsFile"} == r[0].keys()
 
 
 params = [
