Address code review comments

ark/mibi/qc_comp.py

@@ -3,6 +3,7 @@
 import numpy as np
 import pandas as pd
 import matplotlib.pyplot as plt
+from requests.exceptions import HTTPError
 from scipy.ndimage import gaussian_filter
 import seaborn as sns
 from shutil import rmtree
@@ -34,11 +35,14 @@ def create_mibitracker_request_helper(email, password):
             A request helper module instance to access a user's MIBItracker info
     """
 
-    return MibiRequests(settings.MIBITRACKER_BACKEND, email, password)
+    try:
+        return MibiRequests(settings.MIBITRACKER_BACKEND, email, password)
+    except HTTPError:
+        print("Invalid MIBItracker email or password provided")
 
 
 def download_mibitracker_data(email, password, run_name, run_label, base_dir, tiff_dir,
-                              xml_name, img_sub_folder=None, fovs=None, channels=None):
+                              img_sub_folder=None, fovs=None, channels=None):
     """Download a specific run's image data off of MIBITracker
     in an `ark` compatible directory structure
 
@@ -55,16 +59,18 @@ def download_mibitracker_data(email, password, run_name, run_label, base_dir, ti
             Where to place the created `tiff_dir`
         tiff_dir (str):
             The name of the data directory in `base_dir` to write the run's image data to
-        xml_name (str):
-            The name of the XML file to save the run metadata to in `data_dir`.
-            Needs to be suffixed by `.xml`
         img_sub_folder (str):
             If specified, the subdirectory inside each FOV folder in `data_dir` to place
             the image data into
         fovs (list):
             A list of FOVs to subset over. If `None`, uses all FOVs.
         channels (lsit):
             A list of channels to subset over. If `None`, uses all channels.
+
+    Returns:
+        list:
+            A list of tuples containing (point name, point id), sorted by point id.
+            This defines the run acquisition order needed for plotting the QC graphs.
     """
 
     # verify that base_dir provided exists
@@ -78,6 +84,10 @@ def download_mibitracker_data(email, password, run_name, run_label, base_dir, ti
     # NOTE: there will only be one entry in the 'results' key with run_name and run_label specified
     run_info = mr.search_runs(run_name, run_label)
 
+    # if no results are returned, invalid run_name and/or run_label provided
+    if len(run_info['results']) == 0:
+        raise ValueError('No data found for run_name %s and run_label %s' % (run_name, run_label))
+
     # extract the name of the FOVs and their associated internal IDs
     run_fov_names = [img['number'] for img in run_info['results'][0]['imageset']['images']]
     run_fov_ids = [img['id'] for img in run_info['results'][0]['imageset']['images']]
@@ -106,26 +116,20 @@ def download_mibitracker_data(email, password, run_name, run_label, base_dir, ti
         mibitracker_run_chans=run_channels
     )
 
-    # download the run metadata
-    run_metadata = mr.download_file(
-        os.path.join(run_info['results'][0]['path'], run_info['results'][0]['xml'])
-    )
-
     # if the desired tiff_dir exists, remove it
     if os.path.exists(os.path.join(base_dir, tiff_dir)):
         rmtree(os.path.join(base_dir, tiff_dir))
 
     # make the image directory
     os.mkdir(os.path.join(base_dir, tiff_dir))
 
-    # write the run metadata as XML to specified xml_name in tiff_dir
-    with open(os.path.join(base_dir, tiff_dir, xml_name), 'wb') as outfile:
-        outfile.write(run_metadata.read())
-
     # ensure sub_folder gets set to "" if img_sub_folder is None (for os.path.join convenience)
     if not img_sub_folder:
         img_sub_folder = ""
 
+    # define the run order list to return
+    run_order = []
+
     # iterate over each FOV of the run
     for img in run_info['results'][0]['imageset']['images']:
         # if the image fov name is not specified, move on
@@ -155,6 +159,11 @@ def download_mibitracker_data(email, password, run_name, run_label, base_dir, ti
                 chan_data
             )
 
+        # append the run name and run id to the list
+        run_order.append((img['number'], img['id']))
+
+    return run_order
+
 
 def compute_nonzero_mean_intensity(image_data):
     """Compute the nonzero mean of a specific fov/chan pair

ark/mibi/qc_comp_test.py

@@ -1,5 +1,6 @@
 import numpy as np
 import pandas as pd
+from requests.exceptions import HTTPError
 import xarray as xr
 
 from ark.mibi.mibitracker_utils import MibiRequests
@@ -13,14 +14,17 @@
 import tempfile
 
 
+RUN_POINT_NAMES = ['Point%d' % i for i in range(1, 13)]
+RUN_POINT_IDS = list(range(661, 673))
+
 # NOTE: all fovs and all channels will be tested in the example_qc_metric_eval notebook test
 FOVS_CHANS_TEST_MIBI = [
-    (None, ['CCL8', 'CD11b'], None),
-    (None, ['CCL8', 'CD11b'], "TIFs"),
-    (['Point1'], None, None),
-    (['Point1'], None, "TIFs"),
-    (['Point1'], ['CCL8', 'CD11b'], None),
-    (['Point1'], ['CCL8', 'CD11b'], "TIFs")
+    (None, ['CCL8', 'CD11b'], None, RUN_POINT_NAMES, RUN_POINT_IDS),
+    (None, ['CCL8', 'CD11b'], "TIFs", RUN_POINT_NAMES, RUN_POINT_IDS),
+    (['Point1'], None, None, RUN_POINT_NAMES[0:1], RUN_POINT_IDS[0:1]),
+    (['Point1'], None, "TIFs", RUN_POINT_NAMES[0:1], RUN_POINT_IDS[0:1]),
+    (['Point1'], ['CCL8', 'CD11b'], None, RUN_POINT_NAMES[0:1], RUN_POINT_IDS[0:1]),
+    (['Point1'], ['CCL8', 'CD11b'], "TIFs", RUN_POINT_NAMES[0:1], RUN_POINT_IDS[0:1])
 ]
 
 
@@ -42,19 +46,33 @@
 
 
 def test_create_mibitracker_request_helper():
+    # error check: bad email and/or password provided
+    mr = qc_comp.create_mibitracker_request_helper('bad_email', 'bad_password')
+    assert mr is None
+
     # test creation works (just test the correct type returned)
     mr = qc_comp.create_mibitracker_request_helper(MIBITRACKER_EMAIL, MIBITRACKER_PASSWORD)
     assert type(mr) == MibiRequests
 
 
-@pytest.mark.parametrize("test_fovs,test_chans,test_sub_folder", FOVS_CHANS_TEST_MIBI)
-def test_download_mibitracker_data(test_fovs, test_chans, test_sub_folder):
-    # error checks
+@pytest.mark.parametrize(
+    "test_fovs,test_chans,test_sub_folder,actual_points,actual_ids",
+    FOVS_CHANS_TEST_MIBI
+)
+def test_download_mibitracker_data(test_fovs, test_chans, test_sub_folder,
+                                   actual_points, actual_ids):
     with tempfile.TemporaryDirectory() as temp_dir:
-        # bad base_dir provided
+        # error check: bad base_dir provided
         with pytest.raises(FileNotFoundError):
             qc_comp.download_mibitracker_data('', '', '', '', 'bad_base_dir', '', '')
 
+        # error check: bad run_name and/or run_label provided
+        with pytest.raises(ValueError):
+            qc_comp.download_mibitracker_data(
+                MIBITRACKER_EMAIL, MIBITRACKER_PASSWORD, 'bad_run_name', 'bad_run_label',
+                temp_dir, '', ''
+            )
+
         # bad fovs provided
         with pytest.raises(ValueError):
             qc_comp.download_mibitracker_data(
@@ -75,11 +93,11 @@ def test_download_mibitracker_data(test_fovs, test_chans, test_sub_folder):
         os.mkdir(os.path.join(temp_dir, 'sample_tiff_dir'))
 
         # run the data
-        qc_comp.download_mibitracker_data(
+        run_order = qc_comp.download_mibitracker_data(
             MIBITRACKER_EMAIL, MIBITRACKER_PASSWORD,
             MIBITRACKER_RUN_NAME, MIBITRACKER_RUN_LABEL,
-            temp_dir, 'sample_tiff_dir', 'sample_metadata.xml',
-            img_sub_folder=test_sub_folder, fovs=test_fovs, channels=test_chans
+            temp_dir, 'sample_tiff_dir', img_sub_folder=test_sub_folder,
+            fovs=test_fovs, channels=test_chans
         )
 
         # for testing purposes, set test_fovs and test_chans to all fovs and channels
@@ -103,9 +121,6 @@ def test_download_mibitracker_data(test_fovs, test_chans, test_sub_folder):
         # get the contents of tiff_dir
         tiff_dir_contents = os.listdir(os.path.join(temp_dir, 'sample_tiff_dir'))
 
-        # assert sample_metadata.xml was created
-        assert 'sample_metadata.xml' in tiff_dir_contents
-
         # assert all the fovs are contained in the dir
         tiff_dir_fovs = [d for d in tiff_dir_contents if
                          os.path.isdir(os.path.join(temp_dir, 'sample_tiff_dir', d))]
@@ -128,6 +143,13 @@ def test_download_mibitracker_data(test_fovs, test_chans, test_sub_folder):
                 provided_channels=test_chans
             )
 
+        # assert that the run order created is correct for both points and ids
+        run_fov_names = [ro[0] for ro in run_order]
+        run_fov_ids = [ro[1] for ro in run_order]
+
+        assert run_fov_names == actual_points
+        assert run_fov_ids == actual_ids
+
 
 def test_compute_nonzero_mean_intensity():
     sample_img_arr = np.array([[0, 1, 2], [3, 0, 0], [0, 4, 5]])

ark/utils/notebooks_test_utils.py

@@ -146,7 +146,6 @@ def qc_notebook_setup(tb, base_dir, tiff_dir, sub_dir=None, fovs=None, chans=Non
         base_dir = '%s'
         tiff_dir = '%s'
         img_sub_folder = %s
-        xml_name = 'sample_mibitracker_metadata.xml'
     """ % (base_dir, tiff_dir, img_sub_folder)
     tb.inject(data_paths, after='set_data_info')
 

templates/example_qc_metric_eval.ipynb

@@ -65,8 +65,7 @@
     "\n",
     "* `base_dir`: where the created image directory (`tiff_dir`) will be placed\n",
     "* `tiff_dir`: the name of the image directory to store the MIBItracker download\n",
-    "* `img_sub_folder`: if you wish to specify an additional subfolder within each FOV folder to store the images. Set to `None` if you do not want one.\n",
-    "* `xml_name`: the name of the XML run metadata file, will be stored in the root of `base_dir`."
+    "* `img_sub_folder`: if you wish to specify an additional subfolder within each FOV folder to store the images. Set to `None` if you do not want one."
    ]
   },
   {
@@ -83,8 +82,7 @@
     "# set info related\n",
     "base_dir = '../data/example_dataset'\n",
     "tiff_dir = 'example_mibitracker_data'\n",
-    "img_sub_folder = \"TIFs\"\n",
-    "xml_name = 'sample_mibitracker_metadata.xml'"
+    "img_sub_folder = \"TIFs\""
    ]
   },
   {
@@ -121,6 +119,14 @@
     "### Download data off MIBItracker"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "id": "b5f28384-85d8-460c-901b-ec2dfa27afbc",
+   "metadata": {},
+   "source": [
+    "Note: this returns the run acquisition order the FOVs were prorcessed in"
+   ]
+  },
   {
    "cell_type": "code",
    "execution_count": null,
@@ -132,14 +138,13 @@
    },
    "outputs": [],
    "source": [
-    "qc_comp.download_mibitracker_data(\n",
+    "run_order = qc_comp.download_mibitracker_data(\n",
     "    email,\n",
     "    password,\n",
     "    run_name,\n",
     "    run_label,\n",
     "    base_dir,\n",
     "    tiff_dir,\n",
-    "    xml_name,\n",
     "    img_sub_folder,\n",
     "    fovs,\n",
     "    channels\n",
@@ -291,6 +296,14 @@
     "### Visualize QC metrics"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "id": "7555a724-94a6-424b-90f9-2552b2ecb735",
+   "metadata": {},
+   "source": [
+    "Note: the x-axis (FOV labels) of each graph is ordered in the FOV acquisition order"
+   ]
+  },
   {
    "cell_type": "code",
    "execution_count": null,
@@ -310,7 +323,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 14,
+   "execution_count": 13,
    "id": "339f6ea9-1cdb-4b39-bc3b-72267820189c",
    "metadata": {
     "tags": [
@@ -345,7 +358,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 15,
+   "execution_count": 14,
    "id": "53411f27-c351-4442-a1a0-9417c99c316b",
    "metadata": {
     "tags": [
@@ -380,7 +393,7 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 16,
+   "execution_count": 15,
    "id": "864641b2-0d57-4d0a-a991-e149cd024282",
    "metadata": {
     "tags": [