Introduce multiqueries for dataset lookup #33 #230
Conversation
Codecov Report
@@ Coverage Diff @@
## main #230 +/- ##
==========================================
+ Coverage 98.46% 98.47% +0.01%
==========================================
Files 45 45
Lines 2211 2233 +22
Branches 276 282 +6
==========================================
+ Hits 2177 2199 +22
Misses 19 19
Partials 15 15
Continue to review full report at Codecov.
|
terracotta/drivers/mysql.py
Outdated
| @@ -354,10 +355,19 @@ def get_datasets(self, where: Mapping[str, str] = None, | |||
| if not all(key in self.key_names for key in where.keys()): | |||
| raise exceptions.InvalidKeyError('Encountered unrecognized keys in ' | |||
| 'where clause') | |||
| where_fragment = ' AND '.join([f'{key}=%s' for key in where.keys()]) | |||
| where = { | |||
| key: value if isinstance(value, list) else [value] | |||
There was a problem hiding this comment.
I would prefer a
if isinstance(value, str):
value = [value]
terracotta/drivers/mysql.py
Outdated
| for key, value in where.items() | ||
| } | ||
| conditions = [ | ||
| '(%s)' % ' OR '.join([f'{key}=%s'] * len(value)) | ||
| for key, value in where.items() | ||
| ] | ||
| values = list(itertools.chain(*where.values())) | ||
| where_fragment = ' AND '.join(conditions) |
There was a problem hiding this comment.
This is going a bit too far with the comprehensions. I think a straightforward loop might be more readable?
terracotta/drivers/sqlite.py
Outdated
| where = { | ||
| key: value if isinstance(value, list) else [value] | ||
| for key, value in where.items() | ||
| } | ||
| conditions = [ | ||
| '(%s)' % ' OR '.join([f'{key}=?'] * len(value)) | ||
| for key, value in where.items() | ||
| ] | ||
| values = list(itertools.chain(*where.values())) | ||
| where_fragment = ' AND '.join(conditions) |
There was a problem hiding this comment.
A delicate detail of style in code bases - the cases in which list comprehensions are used and where not. 😄I think we generally keep it down to very simple cases. Readability of the code is the property to optimize for.
For readability, I also prefer format strings over the old %s notation...
Make it possible to query datasets like
/datasets?year=[2016,2018]&band=02. This closes #33.The parsing of lists of values is implemented rather naively, but it works.
The
driver.get_datasets()method now acceptswhere: Mapping[str, Union[str, List[str]]]. Handling both cases (conditions either string or list of strings) have introduced some somewhat convoluted code to create the SQL queries, but I think it's about as good as it gets without a larger rewrite.