doc: Expand the documentation for OffTargetDetector
#46
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
=======================================
Coverage 97.37% 97.37%
=======================================
Files 25 25
Lines 1598 1598
Branches 302 302
=======================================
Hits 1556 1556
Misses 23 23
Partials 19 19 ☔ View full report in Codecov by Sentry. |
| 1. Alignment (off-target hit detection): `ref`, `executable`, `threads`, | ||
| `three_prime_region_length`, `max_mismatches_in_three_prime_region`, `max_mismatches`, | ||
| and `max_primer_hits`. | ||
| 2. Filtering of individual primers: `max_primer_hits`. | ||
| 3. Checking of primer pairs: `max_primer_hits`, `min_primer_pair_hits`, | ||
| `max_primer_pair_hits`, and `max_amplicon_size`. |
There was a problem hiding this comment.
I thought it'd be helpful to have the parameters grouped by the function(s) that use them. I feel like there could be a better way to organize/present this, lmk if you have any suggestions
There was a problem hiding this comment.
I agree this is helpful. I'm happy to come back later and expand this further too.
| @@ -150,6 +165,19 @@ def __init__( | |||
| executable: str | Path = "bwa", | |||
| ) -> None: | |||
| """ | |||
| Initialize an [[OffTargetDetector]]. | |||
There was a problem hiding this comment.
Can the primer pair related parameters be given defaults? I don't need them for checking single primers.
There was a problem hiding this comment.
I have been wondering something similar - if it's possible to provide defaults for more of these parameters, or if the features should be decoupled
| **Note**: The reverse complement of each primer sequence is used for mapping. The `query` | ||
| sequence in each `BwaResult` will match the input primer sequence, as will the sequences | ||
| used as keys in the output dictionary. However, the coordinates reported in each `BwaHit` | ||
| associated with a result will correspond to complementary sequence. |
There was a problem hiding this comment.
Is this sufficiently clear?
There was a problem hiding this comment.
It is, but I think this function should be private.
msto
force-pushed
the
ms_doc-off-target
branch
from
382b334 to
1c99e9a
Compare
msto
force-pushed
the
ms_doc-off-target
branch
from
1c99e9a to
b6a2bb6
Compare
| 1. Alignment (off-target hit detection): `ref`, `executable`, `threads`, | ||
| `three_prime_region_length`, `max_mismatches_in_three_prime_region`, `max_mismatches`, | ||
| and `max_primer_hits`. | ||
| 2. Filtering of individual primers: `max_primer_hits`. | ||
| 3. Checking of primer pairs: `max_primer_hits`, `min_primer_pair_hits`, | ||
| `max_primer_pair_hits`, and `max_amplicon_size`. |
There was a problem hiding this comment.
I agree this is helpful. I'm happy to come back later and expand this further too.
| **Note**: The reverse complement of each primer sequence is used for mapping. The `query` | ||
| sequence in each `BwaResult` will match the input primer sequence, as will the sequences | ||
| used as keys in the output dictionary. However, the coordinates reported in each `BwaHit` | ||
| associated with a result will correspond to complementary sequence. |
There was a problem hiding this comment.
It is, but I think this function should be private.
Co-authored-by: Tim Fennell <[email protected]>
Co-authored-by: Tim Fennell <[email protected]>
I added documentation to explain how the class can be used to filter primers vs primer pairs, and the behavior of each filtering method.
I also added
ArgsandReturnblocks to the public methods.