Merge pull request #585 from fedarko/master

Client-side specimen ID matching

.coveragerc

@@ -2,13 +2,15 @@
 branch = True
 omit =
     */tests*
+    */test*
     */__init__.py
     labcontrol/_version.py
     versioneer.py
 
 [report]
 omit =
     */tests*
+    */test*
     */__init__.py
     labcontrol/_version.py
     versioneer.py

README.md

@@ -1,4 +1,6 @@
 # LabControl
+[![Build Status](https://travis-ci.org/biocore/LabControl.svg?branch=master)](https://travis-ci.org/biocore/LabControl) [![Coverage Status](https://coveralls.io/repos/github/biocore/LabControl/badge.svg?branch=master)](https://coveralls.io/github/biocore/LabControl?branch=master)
+
 lab manager for plate maps and sequence flows
 
 # Install

labcontrol/db/process.py

@@ -296,8 +296,9 @@ def update_well(self, row, col, content):
 
         Returns
         -------
-        str
-            The new contents of the well
+        (str, bool)
+            The str corresponds to the new contents of the well; the bool
+            indicates whether or not the new contents of the well are "ok"
         """
         return self.plate.get_well(row, col).composition.update(content)
 

labcontrol/db/study.py

@@ -190,14 +190,20 @@ def samples(self, term=None, limit=None):
         ----------
         term: str, optional
             If provided, return only the samples that contain the given term
-        limit: int, optional
-            If provided, don't return more than `limit` results
+        limit: str, optional
+            If provided, don't return more than `int(limit)` results
 
         Returns
         -------
         list of str
             Returns tube identifiers if the `specimen_id_column` has been set
             (in Qiita), or alternatively returns the sample identifier.
+
+        Raises
+        ------
+        ValueError
+            If `int(limit)` raises a ValueError or OverflowError, or if
+            `limit` is less than or equal to 0.
         """
 
         # SQL generation moved outside of the with conditional to enhance
@@ -222,7 +228,21 @@ def samples(self, term=None, limit=None):
         else:
             order_by_clause = "order by sample_values->'%s'" % column
 
-        if limit:
+        if limit is not None:
+            # Attempt to cast the limit to an int, and (if that works) verify
+            # that the integer limit is greater than zero
+            try:
+                limit = int(limit)
+            except Exception:
+                # Examples of possible exceptions due to int(limit) failing:
+                # - ValueError is most "common," occurs with int("abc")
+                # - OverflowError occurs with int(float("inf"))
+                # - TypeError occurs with int([1,2,3])
+                # This should catch all of the above and any other exceptions
+                # raised due to int(limit) failing.
+                raise ValueError("limit must be castable to an int")
+            if limit <= 0:
+                raise ValueError("limit must be greater than zero")
             order_by_clause += " limit %d" % limit
 
         if column == 'sample_id':

labcontrol/db/tests/test_study.py

@@ -6,6 +6,7 @@
 # The full license is in the file LICENSE, distributed with this software.
 # ----------------------------------------------------------------------------
 
+from math import floor
 from unittest import main
 
 from labcontrol.db.testing import LabControlTestCase
@@ -65,13 +66,13 @@ def test_samples_with_sample_id(self):
         exp_samples = ['1.SKB1.640202', '1.SKB2.640194', '1.SKB3.640195',
                        '1.SKB4.640189', '1.SKB5.640181', '1.SKB6.640176',
                        '1.SKB7.640196', '1.SKB8.640193', '1.SKB9.640200']
-        self.assertEqual(s.samples(limit=9), exp_samples)
+        self.assertEqual(s.samples(limit='9'), exp_samples)
         exp_samples = ['1.SKB1.640202', '1.SKB2.640194', '1.SKB3.640195',
                        '1.SKB4.640189', '1.SKB5.640181', '1.SKB6.640176',
                        '1.SKB7.640196', '1.SKB8.640193', '1.SKB9.640200']
         self.assertEqual(s.samples('SKB'), exp_samples)
         exp_samples = ['1.SKB1.640202', '1.SKB2.640194', '1.SKB3.640195']
-        self.assertEqual(s.samples('SKB', limit=3), exp_samples)
+        self.assertEqual(s.samples('SKB', limit='3'), exp_samples)
         exp_samples = ['1.SKM1.640183', '1.SKM2.640199', '1.SKM3.640197',
                        '1.SKM4.640180', '1.SKM5.640177', '1.SKM6.640187',
                        '1.SKM7.640188', '1.SKM8.640201', '1.SKM9.640192']
@@ -83,6 +84,89 @@ def test_samples_with_sample_id(self):
         exp_samples = ['1.SKB1.640202', '1.SKD1.640179', '1.SKM1.640183']
         self.assertEqual(s.samples('1.64'), exp_samples)
 
+    def test_samples_with_limit(self):
+        """Unit-tests the `limit` argument of Study.samples() in particular.
+
+        It's worth noting that the `limit` value that StudySamplesHandler.get()
+        uses when calling Study.samples() is actually a string -- this is due
+        to our use of tornado.web.RequestHandler.get_argument().
+        Study.samples() only cares that `int(limit)` succeeds, and is otherwise
+        agnostic to the actual input type of `limit`.
+
+        (For the sake of caution, we test a couple of types besides purely
+        `str` values within this function.)
+        """
+        s = Study(1)
+        all_samples = ['1.SKB1.640202', '1.SKB2.640194', '1.SKB3.640195',
+                       '1.SKB4.640189', '1.SKB5.640181', '1.SKB6.640176',
+                       '1.SKB7.640196', '1.SKB8.640193', '1.SKB9.640200',
+                       '1.SKD1.640179', '1.SKD2.640178', '1.SKD3.640198',
+                       '1.SKD4.640185', '1.SKD5.640186', '1.SKD6.640190',
+                       '1.SKD7.640191', '1.SKD8.640184', '1.SKD9.640182',
+                       '1.SKM1.640183', '1.SKM2.640199', '1.SKM3.640197',
+                       '1.SKM4.640180', '1.SKM5.640177', '1.SKM6.640187',
+                       '1.SKM7.640188', '1.SKM8.640201', '1.SKM9.640192']
+        # Check cases where the limit is valid but doesn't actually result in
+        # any filtering being done.
+        self.assertEqual(s.samples(), all_samples)
+        for i in [27, 28, 50, 100, 10000]:
+            self.assertEqual(s.samples(limit=i), all_samples)
+            self.assertEqual(s.samples(limit=str(i)), all_samples)
+        # limit=None is the default, but we check it here explicitly anyway.
+        self.assertEqual(s.samples(limit=None), all_samples)
+
+        # Check *all* limit values in the inclusive range [1, 27] -- these
+        # should, well, limit the output list of samples accordingly
+        for i in range(1, len(all_samples)):
+            self.assertEqual(s.samples(limit=i), all_samples[:i])
+            self.assertEqual(s.samples(limit=str(i)), all_samples[:i])
+
+        float_limits_to_test = [1.0, 1.2, 3.0, 27.0, 29.1, 1000.0]
+        str_of_float_limits_to_test = [str(f) for f in float_limits_to_test]
+
+        # Test that various not-castable-to-a-base-10-int inputs don't work
+        # (This includes string representations of floats, e.g. "1.0", since
+        # such a string is not a valid "integer literal" -- see
+        # https://docs.python.org/3/library/functions.html#int.
+        uncastable_limits_to_test = [
+            [1, 2, 3], "abc", "gibberish", "ten", (1,), "0xBEEF", "0b10101",
+            "0o123", float("inf"), float("-inf"), float("nan"), "None", "inf"
+        ]
+        for i in uncastable_limits_to_test + str_of_float_limits_to_test:
+            with self.assertRaisesRegex(
+                ValueError, "limit must be castable to an int"
+            ):
+                s.samples(limit=i)
+
+        # Calling int(x) where x is a float just truncates x "towards zero"
+        # according to https://docs.python.org/3/library/functions.html#int.
+        #
+        # This behavior is tested, but it should never happen (one, because
+        # as of writing Study.samples() is only called with a string limit
+        # value, and two because I can't imagine why someone would pass a float
+        # in for the "limit" argument).
+        for i in float_limits_to_test:
+            self.assertEqual(s.samples(limit=i), all_samples[:floor(i)])
+
+        # Check that limits <= 0 cause an error
+        nonpositive_limits = [0, -1, -2, -27, -53, -100]
+        for i in nonpositive_limits:
+            with self.assertRaisesRegex(
+                ValueError, "limit must be greater than zero"
+            ):
+                s.samples(limit=i)
+            with self.assertRaisesRegex(
+                ValueError, "limit must be greater than zero"
+            ):
+                s.samples(limit=str(i))
+
+        # Check evil corner case where the limit is nonpositive and not
+        # castable to an int (this should fail first on the castable check)
+        with self.assertRaisesRegex(
+            ValueError, "limit must be castable to an int"
+        ):
+            s.samples(limit="-1.0")
+
     def test_samples_with_specimen_id(self):
         s = Study(1)
 

labcontrol/gui/handlers/study.py

@@ -50,10 +50,17 @@ def get(self, study_id):
         try:
             study = Study(int(study_id))
             term = self.get_argument('term', None)
-            res = list(study.samples(term, limit=20))
+            limit = self.get_argument('limit', None)
+            # If the specified limit is somehow invalid, study.samples() will
+            # raise a ValueError. In this case, we'll set a 400 ("Bad Request")
+            # status code.
+            res = list(study.samples(term, limit))
             self.write(json_encode(res))
         except LabControlUnknownIdError:
             self.set_status(404)
+        except ValueError:
+            # These will be raised if the limit passed is invalid
+            self.set_status(400)
         self.finish()
 
 

labcontrol/gui/js_tests/labcontrol_tests.js

@@ -79,3 +79,32 @@ QUnit.module("clippingForPlateType", function(hooks) {
     assert.equal(clippingForPlateType("not a valid type"), 10000);
   });
 });
+
+QUnit.module("getSubstringMatches", function(hooks) {
+  QUnit.test("common usage", function(assert) {
+    assert.deepEqual(getSubstringMatches("w", ["wahoo", "walrus"]), [
+      "wahoo",
+      "walrus"
+    ]);
+    assert.deepEqual(getSubstringMatches("h", ["wahoo", "walrus"]), ["wahoo"]);
+    assert.equal(getSubstringMatches("z", ["wahoo", "walrus"]).length, 0);
+    assert.deepEqual(getSubstringMatches("1234f", ["01234f", "01234"]), [
+      "01234f"
+    ]);
+    assert.deepEqual(
+      getSubstringMatches("abc", ["abc", "ABCDEF", "AbCdE", "", "DEF"]),
+      ["abc", "ABCDEF", "AbCdE"]
+    );
+  });
+  QUnit.test("empty query text and/or empty strings in array", function(
+    assert
+  ) {
+    assert.equal(getSubstringMatches("", ["wahoo", "walrus"]).length, 0);
+    assert.equal(getSubstringMatches("", ["wahoo", "walrus", ""]).length, 0);
+    assert.equal(getSubstringMatches("abc", ["wahoo", "walrus", ""]).length, 0);
+    assert.deepEqual(getSubstringMatches("w", ["wahoo", "walrus", ""]), [
+      "wahoo",
+      "walrus"
+    ]);
+  });
+});

labcontrol/gui/static/js/labcontrol.js

@@ -742,3 +742,37 @@ function createHeatmap(
     });
   });
 }
+
+/**
+ *
+ * Given a query string and an array of strings to search through, returns a
+ * subset of the array containing only strings that contain the query string
+ * (or an empty array, if no such matches are found).
+ *
+ * This search is case insensitive, so "abc" will match with "ABCDEF".
+ *
+ * Note that if queryString is empty (i.e. "") then this won't return any
+ * matches, even if "" is present in stringArray for some reason (since
+ * stringArray should be a list of sample IDs, this should never be the case).
+ *
+ * This function is loosely based on textFilterFeatures() in Qurro:
+ * https://github.com/biocore/qurro/blob/3f4650fb677753e978d971b06794d4790b051d30/qurro/support_files/js/feature_computation.js#L29
+ *
+ * @param {string} queryString The string that will be searched for
+ * @param {array} stringArray Collection of strings to check against the query
+ * @returns {array} Collection of strings in stringArray that include the query
+ *
+ */
+function getSubstringMatches(queryString, stringArray) {
+  if (queryString.length === 0 || stringArray.length === []) {
+    return [];
+  }
+  var queryStringLowerCase = queryString.toLowerCase();
+  var matchingStrings = [];
+  for (var i = 0; i < stringArray.length; i++) {
+    if (stringArray[i].toLowerCase().includes(queryStringLowerCase)) {
+      matchingStrings.push(stringArray[i]);
+    }
+  }
+  return matchingStrings;
+}

labcontrol/gui/static/js/plate.js

@@ -119,7 +119,7 @@ function activate_study(studyId) {
  *
  **/
 function change_plate_configuration() {
-  var pv, $opt;
+  var $opt;
   $opt = $("#plate-conf-select option:selected");
   var plateId = $("#plateName").prop("pm-data-plate-id");
   if (plateId !== undefined) {