SKETCH-2763: Generate complement strand for nucleic acids - #367
SKETCH-2763: Generate complement strand for nucleic acids#367JarrettSJohnson wants to merge 1 commit into
Conversation
fa32ff8 to
b3ad7b4
Compare
ethan-schrodinger
left a comment
There was a problem hiding this comment.
LGTM!
I have some minort comments, but obviously it's up to you whether they are worth implementing.
| * @brief Per-base data needed to build one complement monomer triplet (sugar, | ||
| * base, phosphate). | ||
| * | ||
| * @c orig_idx is the index of the original base this triplet pairs with. | ||
| */ | ||
| struct ComplementPair { |
There was a problem hiding this comment.
Nit: maybe I'm missing something, but I don't know whether ComplementPair is the best name for this, since it's not a pair of anything.
There was a problem hiding this comment.
Fixed — renamed ComplementPair to ComplementNucleotide and renamed its fields to original_base_idx, sugar_symbol, and base_symbol so the type describes the single complement nucleotide it represents.
Signed Codex
…number per monomer, even for nucleic acids (schrodinger#372) - Linked Case: SKETCH-2782 ### Description Based on [this discussion](schrodinger#367 (comment)), I've modified `get_residue_number_for_new_monomer` to use one residue number per monomer, even for nucleic acids. ### Testing Done Added unit tests coverage for the new logic.
b3ad7b4 to
af262d3
Compare
KevKeating
left a comment
There was a problem hiding this comment.
I had a few minor questions, but they can always be addressed later if need be.
| // One undo entry covers the complement chains for every source polymer. | ||
| auto undo_macro = createUndoMacro("Add Complementary Sequence"); | ||
| for (const auto& [polymer_id, base_idxs] : bases_by_polymer) { | ||
| addComplementChainForPolymer(base_idxs); |
There was a problem hiding this comment.
Will this add an empty entry to the undo stack if none of the polymers have any complementable nucleotides?
| if (is_first) { | ||
| addMonomer(complement.sugar_symbol, | ||
| rdkit_extensions::ChainType::RNA, sugar_coord); | ||
| } else { |
There was a problem hiding this comment.
What happens if there's a non-complementable nucleotide in between two complementable nucleotides? Will this create an extra long connection between the two complements?
| * @param bases the selected atoms | ||
| * @return a map from polymer id to the contained base atom indices | ||
| */ | ||
| static std::map<std::string, std::vector<size_t>> |
There was a problem hiding this comment.
These static methods and the ComplementNucleotide struct could probably go in monomeric.h/cpp since they don't seem to need access to anything MolModel-related. That would keep this file a bit cleaner, but I'm okay either way. We can always move them later if we need to access them somewhere else.
Description
This revision offers support for the generation of a complement strand for a selected set of nucleic acids.
Adds a menu option to generate the complement whenever a set of nucleic acids are selected; mixed selections disables the option.
Ordering and pointer invalidation is a bit tricky for building up the sequences in this way, so we employ an index-based strategy to build up the nascent chain for each polymer.
Testing Done
Adds some tests for conditions where menus should show, generation of complement, and undo.