Closed ckikawa closed 6 months ago
@ckikawa, I think your diagnosis of the problem appears correct, but I think wouldn't a better solution just to be to re-define the relevant key of under manual_drops
in the config
to be well
rather than wells
?
That solution would not require changing code (just documentation), and would make it cleaner.
Also, this would not really be a backward-incompatible change as given the problem you identified, no one must be using wells
right now or they would be getting the same error.
So I would suggest testing if you can fix this by just updating wells
to well
in the YAML configuration, and if that works just do a pull request fixing the documentation here rather than changing the code.
@jbloom, unfortunately just changing at the level of the config
will not work because of this block of code, also in process_plate.ipynb
. When I run with manual_drops
key well
instead of wells
, an error is given:
qc_drops = {
"wells": {},
"barcodes": {},
"barcode_wells": {},
"barcode_serum_replicates": {},
"serum_replicates": {},
}
assert set(manual_drops).issubset(
qc_drops
), f"{manual_drops.keys()=}, {qc_drops.keys()}"
------------------
---------------------------------------------------------------------------
AssertionError Traceback (most recent call last)
Cell In[4], line 9
1 qc_drops = {
2 "wells": {},
3 "barcodes": {},
(...)
6 "serum_replicates": {},
7 }
----> 9 assert set(manual_drops).issubset(
10 qc_drops
11 ), f"{manual_drops.keys()=}, {qc_drops.keys()}"
AssertionError: manual_drops.keys()=dict_keys(['well']), dict_keys(['wells', 'barcodes', 'barcode_wells', 'barcode_serum_replicates', 'serum_replicates'])
The dictionary qc_drops
is also used throughout the notebook to add wells, barcodes etc. failing default QC parameters.
So, I think the solution could be either:
elif
statement similar to the one I wrote aboveqc_drops
key from wells
to well
, which would also require small changes in other cells in the process_plate.ipynb
where wells
is keyed OK, in that case I would say go ahead and make the change you suggest as a pull request. Does a similar change need to be made for the barcodes
drop?
Yes, I think so since qc_drops
specifies barcodes
but the counts
dataframe column is named barcode
. I'll do a similar fix for that.
I have specified a
manual_drop
in my config for a specific plate, which looks like this:This causes an assertion error in the following block of code in the
process_plate17.ipynb
notebook. I'm not sure, but I believe this is because themanual_drops
dictionary requires the keywells
(plural!), but the actual column in the counts file is namedwell
(singular!).In this case, this issue could be resolved by adding another
elif
statement to the same cell, something like:Happy to try to tackle this @jbloom, unless you'd prefer to look at it!