search

broadinstitute / adapt

A package for designing activity-informed nucleic acid diagnostics for viruses.
MIT License
27 stars 1 forks source link

Add do-not-overlap option #48

Closed priyappillai closed 3 years ago

priyappillai commented 3 years ago

Added an option for users to input the level of overlap that should not be allowed (amplicon, primer, or none, default amplicon). All options have been tested on example FASTA.

priyappillai commented 3 years ago

Looks good. One thing I'm confused about is unit tests, which are explicitly providing True/False to no_overlap -- specifically, the one that passes in the case where no_overlap=True. You should also update the unit tests, and I'd be curious why they're passing.

It looks like there's only one unit test which uses no_overlap=True. The code ends up interpreting this is as no_overlap='none' since it just checks if it's either 'amplicon' or 'primer', and the test doesn't actually check if the result isn't overlapping. Updating tests!

And thanks for catching all the extraneous spaces at the ends of lines!

I set up Sublime to automatically do this on save 😄