biocore / LabControl

lab manager for plate maps and sequence flows
BSD 3-Clause "New" or "Revised" License
2 stars 15 forks source link

PoolListHandler.get would identify pools from future new protocols as amplicon sequencing pools #510

Open AmandaBirmingham opened 5 years ago

AmandaBirmingham commented 5 years ago

The code in PoolListHandler.get identifies amplicon sequencing pools by process of elimination (i.e., any pool that is is not made up of any kind of known pool plate is treated as an amplicon sequencing pool):

https://github.com/jdereus/labman/blob/a5308e9c26a0344fd30e1c0c5e50c9a7d212fbef/labcontrol/gui/handlers/pool.py#L23-L39

This issue must be addressed before adding any new protocols or pool types to the system, because such an addition could lead this code to incorrectly identify pools based on these new protocols as amplicon sequencing pools.

LIkely makes sense to fix this in conjunction with #509 Centralize and harden identification of pool component types

AmandaBirmingham commented 5 years ago

This issue captures one of the code smells from #500

charles-cowart commented 5 years ago

IMHO, we should avoid fall-throughs and explicitly define all valid paths, so we can defensively assert against the currently invalid ones, make it easier to add new functionality, etc. Tentatively adding to Full Launch, as it's needed to add the MiniPCR protocol.