Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client-side specimen ID matching #585

Merged
merged 46 commits into from
Sep 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
be1233f
ENH: Expose limit in StudySamplesHandler #520 #553
fedarko Aug 21, 2019
e00fcd0
MAINT: add get_active_samples(); abstract code
fedarko Aug 22, 2019
b69f649
Merge branch 'master' of https://github.com/jdereus/LabControl
fedarko Aug 22, 2019
f7fce69
Merge branch 'master' of https://github.com/jdereus/LabControl
fedarko Aug 23, 2019
129e30b
Merge branch 'master' of https://github.com/jdereus/LabControl
fedarko Aug 26, 2019
0421acb
ENH: Prototype impl. of multi-select #520 #553
fedarko Aug 27, 2019
1920405
MAINT: remove now-unneeded variable declaration
fedarko Aug 27, 2019
1f38eb4
MAINT: Apply auto-matching regardless of ms c.box
fedarko Aug 28, 2019
00b4abd
Merge branch 'master' of https://github.com/jdereus/LabControl
fedarko Aug 28, 2019
28e979c
MAINT: pass active study ID to patchWell
fedarko Aug 29, 2019
08226b5
Merge branch 'master' of https://github.com/jdereus/LabControl
fedarko Aug 29, 2019
d81253d
DOC: Add comments re: multi-matching details
fedarko Aug 30, 2019
967297e
ENH: show yellow box on indet. wells *initially*
fedarko Aug 30, 2019
cf36c66
DOC: Add TODO comment re: patchWell efficiency
fedarko Aug 30, 2019
948dd19
DOC: add comment re: a formerly-missing return val
fedarko Aug 30, 2019
0f3d2da
HACK: Document how the indet. color is temporary
fedarko Aug 30, 2019
9a5eefc
MAINT: Split getSubstringMatches into lc.js & test
fedarko Aug 30, 2019
c5de9aa
MAINT: remove old console.log statements
fedarko Aug 30, 2019
77cb5a6
MAINT: Add onRejected funcs throughout plateViewer
fedarko Sep 6, 2019
ed55343
MAINT: show responseText when applicable in alerts
fedarko Sep 6, 2019
2c37399
TST: Fix test_get_study_samples_handler re changes
fedarko Sep 6, 2019
26f82db
TST: Add missing response code check in a test
fedarko Sep 7, 2019
bea54e0
TST: Explicitly test & improve study samples limit
fedarko Sep 7, 2019
ea536f3
TST: Fix error in exp output + beef up test cases
fedarko Sep 7, 2019
b8e433b
DOC: Add Travis badge to README [ci skip]
fedarko Sep 7, 2019
f70a0ba
DOC: whoops, add a linebreak after Travis badge
fedarko Sep 7, 2019
75e0978
DOC: Add Coveralls badge to README
fedarko Sep 7, 2019
10ef1ef
BUG: Remove labcontrol/gui/test* from coverage
fedarko Sep 8, 2019
4014bad
STY: Change return signature to "explicit tuple"
fedarko Sep 10, 2019
1b558b4
MAINT: Don't explicitly check limit in a handler
fedarko Sep 10, 2019
289fcd4
DOC: updates -> update
fedarko Sep 10, 2019
1037e17
DOC: Fix error in patchWell studyID description
fedarko Sep 10, 2019
4b9752a
BUG: remove duplicate hidden IDs in plate template
fedarko Sep 10, 2019
cf3a841
MAINT: Make study limit-checking more pythonic
fedarko Sep 10, 2019
f7b6d06
TST: test negative float limit case from handler
fedarko Sep 10, 2019
661c8cc
BUG/TST: catch TypeError on int(limit)
fedarko Sep 11, 2019
f313223
DOC: Fix description of getSubstringMatches return
fedarko Sep 11, 2019
ad932ee
DOC: Remove comment that is superseded by #591
fedarko Sep 11, 2019
fb58786
DOC: Improve plateViewer docs/error messages
fedarko Sep 11, 2019
5e68a0d
DOC: Copy a comment to where code has been copied
fedarko Sep 11, 2019
7e4db61
DOC: possiblyNewContent -> possiblyMatchedContent
fedarko Sep 11, 2019
3fab6ed
DOC: add reference from comment to #592
fedarko Sep 11, 2019
827ac67
DOC: Update modifyWell() docs re reviewer comments
fedarko Sep 11, 2019
97f020d
TST: For consistency, propEqual -> deepEqual
fedarko Sep 11, 2019
9403705
MAINT: More general error-handling of int(limit)
fedarko Sep 11, 2019
c0db25b
TST: for completeness' sake, test str(<= 0) limits
fedarko Sep 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
branch = True
omit =
*/tests*
*/test*
fedarko marked this conversation as resolved.
Show resolved Hide resolved
*/__init__.py
labcontrol/_version.py
versioneer.py

[report]
omit =
*/tests*
*/test*
*/__init__.py
labcontrol/_version.py
versioneer.py
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
5 changes: 3 additions & 2 deletions labcontrol/db/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
26 changes: 23 additions & 3 deletions labcontrol/db/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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':
Expand Down
88 changes: 86 additions & 2 deletions labcontrol/db/tests/test_study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']
Expand All @@ -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)

Expand Down
9 changes: 8 additions & 1 deletion labcontrol/gui/handlers/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down
29 changes: 29 additions & 0 deletions labcontrol/gui/js_tests/labcontrol_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]);
});
});
34 changes: 34 additions & 0 deletions labcontrol/gui/static/js/labcontrol.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
fedarko marked this conversation as resolved.
Show resolved Hide resolved
* 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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any objection to moving toward the default use of let instead of var? See https://stackoverflow.com/q/762011

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with that (so long as we're ok with only supporting ES6-capable browsers). To clarify, do you just mean using let instead of var from here on, or actively porting the old code to use let?

grep -ri "var " labcontrol/gui/static/js/ labcontrol/gui/js_tests/*.js | wc -l finds 171 hits and grep -ri "let " labcontrol/gui/static/js/ labcontrol/gui/js_tests/*.js | wc -l finds 28 hits, so the latter option might take some time :)

var matchingStrings = [];
for (var i = 0; i < stringArray.length; i++) {
if (stringArray[i].toLowerCase().includes(queryStringLowerCase)) {
matchingStrings.push(stringArray[i]);
}
}
return matchingStrings;
}
2 changes: 1 addition & 1 deletion labcontrol/gui/static/js/plate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading