Conversation
…d to check tests Add recombinase assembly algorithm for attB/attP
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #564 +/- ##
==========================================
+ Coverage 93.56% 93.80% +0.23%
==========================================
Files 37 38 +1
Lines 5302 5394 +92
Branches 748 757 +9
==========================================
+ Hits 4961 5060 +99
+ Misses 267 261 -6
+ Partials 74 73 -1
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements generalized recombinase functionality for pydna, addressing issue #435 and building upon PR #496. The changes introduce a new recombinase module that provides a flexible framework for simulating site-specific recombination reactions, and refactors the existing Gateway cloning functionality to use this new module.
Changes:
- Added new
src/pydna/recombinase.pymodule withRecombinaseandRecombinaseCollectionclasses for generic recombinase operations - Refactored
src/pydna/gateway.pyto use the new recombinase infrastructure, converting site definitions to use lowercase notation for homology cores - Added
recombinase_integration()andrecombinase_excision()functions tosrc/pydna/assembly2.py - Added comprehensive test suite in
tests/test_module_recombinase.pycovering various scenarios - Added
RecombinaseSourceclass tosrc/pydna/opencloning_models.pyfor provenance tracking - Updated test expectations in
tests/test_module_assembly2.pyto reflect new site ordering behavior
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pydna/recombinase.py | New module implementing generic recombinase functionality with support for site finding, annotation, and overlap detection |
| src/pydna/gateway.py | Refactored to use new Recombinase classes, simplified by delegating to RecombinaseCollection |
| src/pydna/assembly2.py | Added recombinase_integration and recombinase_excision functions, plus minor fixes to SingleFragmentAssembly |
| src/pydna/opencloning_models.py | Added RecombinaseSource class for tracking recombinase-based assemblies |
| tests/test_module_recombinase.py | Comprehensive test suite for the new recombinase module |
| tests/test_module_assembly2.py | Updated expected error message to reflect new site ordering |
Comments suppressed due to low confidence (3)
src/pydna/assembly2.py:1974
- The comment states "Remove matches where the whole sequence matches", but this filtering removes matches where the match length equals the fragment length. For recombinase reactions, this makes sense (you don't want the entire sequence to match itself), but it's worth noting this is a semantic change from the previous behavior. Consider adding a more detailed comment explaining why full-length matches need to be filtered out for recombinase assembly, as this might not be immediately obvious to future maintainers.
# Remove matches where the whole sequence matches
matches = [match for match in matches if match[2] != len(frag)]
src/pydna/recombinase.py:140
- Missing space in error message. Should be "Recombinase recognition site is not in the expected format. Expected format:" with a space before "Expected".
raise ValueError(
"Recombinase recognition site is not in the expected format."
"Expected format: [A-Z]+[a-z]+[A-Z]+, e.g. 'ATGCCCTAAaaTT'"
)
src/pydna/recombinase.py:189
- Missing space in error message. Should be "Recombinase recognition sites do not have matching homology cores. Expected" with a space before "Expected".
raise ValueError(
"Recombinase recognition sites do not have matching homology cores."
f"Expected {site1[off1:off1 + len1]} == {site2[off2:off2 + len2]}"
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #435 and completes #496
cc @BjornFJohansson please have a look at the docs of the new module here
It works for attB / attP - like recombinases, but perhaps I am missing something