Conversation
Create Working Branch
wasade
left a comment
wasade
left a comment
There was a problem hiding this comment.
In addition to the uncertainty about why this is needed raised on the other PR, I'm concerned here particularly with the handling of the kit ID and barcodes files
| start_index = result.find("Key") | ||
| if start_index != -1: | ||
| error_message = result[start_index:] | ||
| error_message = error_message[:44] | ||
| else: | ||
| error_message = result |
| for kit_index, row in kits.iterrows(): | ||
| sample_barcodes = row['sample_barcodes'] | ||
| for sample_index in range(len(sample_barcodes)): | ||
| kits.at[kit_index, f'barcode_{sample_index + 1}'] = \ | ||
| sample_barcodes[sample_index] |
There was a problem hiding this comment.
I think this is a logical join or concat operation, could it be re-expressed as that? The iteration and subsequent elementwise calls to .at are inefficient, and though this is an administrative interface, it would disrupt wet lab technicians if the web server is locked up.
|
|
||
| def _read_csv_file(file): | ||
| content = file.read().decode('utf-8-sig') | ||
| return [row[0] for row in csv.reader(io.StringIO(content), |
There was a problem hiding this comment.
Since we're already using Pandas, shouldn't we fallback on its parser?
There was a problem hiding this comment.
Oh this doesn't really "_read_csv_file" but rather just expects an individual string per line?
| kit_ids = _read_csv_file(kit_ids_file) | ||
| if 'barcodes_file' in request.files: | ||
| # User provided barcodes | ||
| barcodes = _read_csv_file(request.files['barcodes_file']) |
There was a problem hiding this comment.
The barcodes are provided in a different file than the kit IDs? What's the schema of these files, and if the kit ID is a key between them, can that be enforced?
There was a problem hiding this comment.
_read_csv_file will produce [list, of, barcodes, ]. A few lines above, it will also produce [list, of, kit, ids, ].
If kit_ids has len() > 1, how is it determined which barcode is associated with which kit?
|
|
||
| with tempfile.NamedTemporaryFile( | ||
| mode='w', | ||
| delete=False, |
There was a problem hiding this comment.
When will the temp file be deleted? Why not just use StringIO or BytesIO and avoid tmp?
| <tr> | ||
| <td colspan="2"> | ||
| <br /> | ||
| Upload comma-separated value file(s) (CSV) |
There was a problem hiding this comment.
This doesn't describe anything about what these files should contain or how they should be structured
3 participants
This PR addresses two related needs:
The corresponding microsetta-private-api PR is biocore/microsetta-private-api#590