diff --git a/.coveragerc b/.coveragerc index 0a556fb0..e59bfaea 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,6 +2,7 @@ branch = True omit = */tests* + */test* */__init__.py labcontrol/_version.py versioneer.py @@ -9,6 +10,7 @@ omit = [report] omit = */tests* + */test* */__init__.py labcontrol/_version.py versioneer.py diff --git a/README.md b/README.md index 1e205842..d0f43ac1 100644 --- a/README.md +++ b/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 diff --git a/labcontrol/db/process.py b/labcontrol/db/process.py index a576f8a4..7b76e175 100644 --- a/labcontrol/db/process.py +++ b/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) diff --git a/labcontrol/db/study.py b/labcontrol/db/study.py index 968be6a0..1e4ee324 100644 --- a/labcontrol/db/study.py +++ b/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': diff --git a/labcontrol/db/tests/test_study.py b/labcontrol/db/tests/test_study.py index 341054c3..92c97b73 100644 --- a/labcontrol/db/tests/test_study.py +++ b/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) diff --git a/labcontrol/gui/handlers/study.py b/labcontrol/gui/handlers/study.py index 59b7895c..34d77ea4 100644 --- a/labcontrol/gui/handlers/study.py +++ b/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() diff --git a/labcontrol/gui/js_tests/labcontrol_tests.js b/labcontrol/gui/js_tests/labcontrol_tests.js index f986175d..0d2df4b9 100644 --- a/labcontrol/gui/js_tests/labcontrol_tests.js +++ b/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" + ]); + }); +}); diff --git a/labcontrol/gui/static/js/labcontrol.js b/labcontrol/gui/static/js/labcontrol.js index 1e65127d..1d6d0b03 100644 --- a/labcontrol/gui/static/js/labcontrol.js +++ b/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; +} diff --git a/labcontrol/gui/static/js/plate.js b/labcontrol/gui/static/js/plate.js index e35f27c6..70309ab3 100644 --- a/labcontrol/gui/static/js/plate.js +++ b/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) { diff --git a/labcontrol/gui/static/js/plateViewer.js b/labcontrol/gui/static/js/plateViewer.js index 28566518..e2726be9 100644 --- a/labcontrol/gui/static/js/plateViewer.js +++ b/labcontrol/gui/static/js/plateViewer.js @@ -286,10 +286,8 @@ PlateViewer.prototype.initialize = function(rows, cols) { // functions like this one. if (this.checked) { that.grid.registerPlugin(that.cellExternalCopyManager); - // console.log("cecm plugin registered"); } else { that.grid.unregisterPlugin(that.cellExternalCopyManager); - // console.log("cecm plugin UNregistered"); } }); // This paradigm inspired by @@ -436,16 +434,17 @@ PlateViewer.prototype.getActiveStudy = function() { /** * - * Modify the contents of a well + * Actually update a well's content in the backend. This code was ripped out + * of PlateViewer.modifyWell(). * * @param {int} row The row of the well being modified * @param {int} col The column of the well being modified * @param {string} content The new content of the well + * @param {string} studyID The output of getActiveStudy() * - **/ -PlateViewer.prototype.modifyWell = function(row, col, content) { - var that = this, - studyID = this.getActiveStudy(); + */ +PlateViewer.prototype.patchWell = function(row, col, content, studyID) { + var that = this; $.ajax({ url: "/process/sample_plating/" + this.processId, @@ -479,6 +478,61 @@ PlateViewer.prototype.modifyWell = function(row, col, content) { }); }; +/** + * + * Modify the contents of a well + * + * @param {int} row The row of the well being modified + * @param {int} col The column of the well being modified + * @param {string} content The new content of the well + * + **/ +PlateViewer.prototype.modifyWell = function(row, col, content) { + var that = this, + studyID = this.getActiveStudy(); + + // Perform automatic matching. Note that this is currently done every time + // modifyWell() is called, even if the user types in something like "blank" + // or even if the user selects something from a dropdown list. (That later + // case could probably be detected in order to avoid doing matching here.) + // + // This functionality has not been stress-tested on large-scale datasets + // (e.g. the American Gut Project) yet; this is a major TODO, as part of + // issue #173 on GitHub. + // + // A major bottleneck here, I think, will be how requests are made on a well- + // by-well basis to the server in patchWell() instead of in batch operations. + var possiblyMatchedContent = content; + // TODO: cache list of active samples so that we don't have to make this + // particular request every time modifyWell() is called. See #592 in GH repo. + get_active_samples().then( + function(sampleIDs) { + // If there is *exactly one* match with an active sample ID, use + // that instead. In any other case (0 matches or > 1 matches), + // manual resolution is required -- so we don't bother changing the + // cell content. + var matchingSamples = getSubstringMatches(content, sampleIDs); + if (matchingSamples.length === 1) { + possiblyMatchedContent = matchingSamples[0]; + safeArrayDelete(that.wellClasses[row][col], "well-indeterminate"); + } else if (matchingSamples.length > 1) { + addIfNotPresent(that.wellClasses[row][col], "well-indeterminate"); + } else { + safeArrayDelete(that.wellClasses[row][col], "well-indeterminate"); + } + that.patchWell(row, col, possiblyMatchedContent, studyID); + }, + function(rejectionReason) { + bootstrapAlert( + "Attempting to get a list of sample IDs in PlateViewer.modifyWell() " + + "failed: " + + rejectionReason, + "danger" + ); + } + ); +}; + /** **/ PlateViewer.prototype.commentWell = function(row, col, comment) { @@ -745,26 +799,65 @@ function autocomplete_search_samples(request, response) { // Perform all the requests to the server var requests = [$.get("/sample/control?term=" + request.term)]; $.each(studyIds, function(index, value) { - requests.push($.get("/study/" + value + "/samples?term=" + request.term)); + requests.push( + $.get("/study/" + value + "/samples?limit=20&term=" + request.term) + ); }); - $.when.apply($, requests).then(function() { - // The nature of arguments change based on the number of requests performed - // If only one request was performed, then arguments only contain the output - // of that request. On the other hand, if there was more than one request, - // then arguments is a list of results - var arg = requests.length === 1 ? [arguments] : arguments; - var samples = []; - $.each(arg, function(index, value) { - samples = samples.concat($.parseJSON(value[0])); - }); - // Format the samples in the way that autocomplete needs - var results = []; - $.each(samples, function(index, value) { - results.push({ label: value, value: value }); - }); - response(results); + $.when.apply($, requests).then( + function() { + // The nature of arguments change based on the number of requests + // performed. If only one request was performed, then arguments only + // contain the output of that request. On the other hand, if there was + // more than one request, then arguments is a list of results + var arg = requests.length === 1 ? [arguments] : arguments; + var samples = merge_sample_responses(arg); + // Format the samples in the way that autocomplete needs + var results = []; + $.each(samples, function(index, value) { + results.push({ label: value, value: value }); + }); + response(results); + }, + // If any of the requests fail, the arguments to this "rejection" + // function match the arguments to the corresponding request's + // "error" function: see https://api.jquery.com/jquery.when/, towards the + // bottom of the page. (So we can just show the user jqXHR.responseText.) + function(jqXHR, textStatus, errorThrown) { + bootstrapAlert( + "Attempting to get sample IDs while filling up the autocomplete " + + "dropdown menu in autocomplete_search_samples() failed: " + + jqXHR.responseText, + "danger" + ); + } + ); +} + +/** + * Given an array of responses of sample IDs (where each element in the array + * is a string representation of an array of sample IDs), returns a single + * array of sample IDs. + * + * This function was created in order to store redundant code between + * autocomplete_search_samples() (where this code was taken from) and + * get_active_samples(). + * + * NOTE that this does not account for cases where there are duplicate + * sample IDs in the input. If for whatever reason a sample ID is repeated in a + * study -- or multiple studies share a sample ID -- then this function won't + * have a problem with that, and will accordingly return an array containing + * duplicate sample IDs. (That should never happen, though.) + * + * @param {Array} responseArray: an array of request(s') output. + * @returns {Array} A list of the sample IDs contained within responseArray. + */ +function merge_sample_responses(responseArray) { + var samples = []; + $.each(responseArray, function(index, value) { + samples = samples.concat($.parseJSON(value[0])); }); + return samples; } /** @@ -789,6 +882,56 @@ function get_active_studies() { return studyIds; } +/** + * Function to retrieve every sample ID associated with every active study. + * + * A lot of this code was based on autocomplete_search_samples() -- however, + * not using a sample count limit or taking into account control types means + * that this function is inherently simpler. + * + * This returns a Promise because this function does a few asynchronous calls + * under the hood (in particular, it issues one request per active study). In + * order to be consistent, a Promise is returned even if no studies are active + * (in this case the Promise will just resolve to []). + * + * @returns {Promise} A Promise that resolves to a list of sample IDs from all + * active studies. + */ +function get_active_samples() { + var studyIDs = get_active_studies(); + if (studyIDs.length > 0) { + // Create an array of all the requests to be made (one request per study) + var requests = []; + $.each(studyIDs, function(index, value) { + requests.push($.get("/study/" + value + "/samples")); + }); + // Wait for each request to be handled, then merge responses to get a list of + // sample IDs from all active studies + // We're going to return a Promise, and this Promise's value will be the + // array of all sample IDs from all active studies. + return $.when.apply($, requests).then( + function() { + // The nature of arguments change based on the number of requests + // performed. If only one request was performed, then arguments only + // contain the output of that request. On the other hand, if there was + // more than one request, then arguments is a list of results + var arg = requests.length === 1 ? [arguments] : arguments; + return merge_sample_responses(arg); + }, + function(jqXHR, textStatus, errorThrown) { + bootstrapAlert( + "Attempting to get a list of sample IDs in get_active_samples() " + + "failed: " + + jqXHR.responseText, + "danger" + ); + } + ); + } else { + return Promise.resolve([]); + } +} + /** * Small widget to add notes and save them to a URI * diff --git a/labcontrol/gui/templates/plate.html b/labcontrol/gui/templates/plate.html index 68307edf..3adec20a 100644 --- a/labcontrol/gui/templates/plate.html +++ b/labcontrol/gui/templates/plate.html @@ -206,15 +206,19 @@

Plate map

-

Specimen ID matches more than one member of the attached studies

+

Specimen ID matches more than one member of the attached + studies; if left unresolved, will show up as "not a member of the + attached studies" when reloading this plate

-