From ed5acac092f239fcb310238a4cf204445fccb78c Mon Sep 17 00:00:00 2001 From: David Doty Date: Sun, 12 Jul 2026 09:26:42 -0700 Subject: [PATCH 1/5] fixes #321 add_half_crossover removes modifications and fixes #320 ensure names preserved with add_half_crossover --- scadnano/scadnano.py | 113 ++++++++- tests/scadnano_tests.py | 529 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 633 insertions(+), 9 deletions(-) diff --git a/scadnano/scadnano.py b/scadnano/scadnano.py index be7ccca..ae75dae 100644 --- a/scadnano/scadnano.py +++ b/scadnano/scadnano.py @@ -8679,8 +8679,26 @@ def add_nick(self, helix: int, offset: int, forward: bool, new_color: bool = Tru # TODO: what if these are extensions or loopouts? domains_before = domains[:order] domains_after = domains[order + 1 :] - domain_left: Domain = Domain(helix, forward, domain_to_remove.start, offset) - domain_right: Domain = Domain(helix, forward, offset, domain_to_remove.end) + # deletions and insertions are keyed by offset, so each goes to whichever of the two new domains + # covers its offset. They must be carried over: they change Domain.dna_length(), so dropping them + # would misalign the DNA sequence and the internal modification indices with the bases they + # describe. (ligate likewise unions them back together.) + domain_left: Domain = Domain( + helix, + forward, + domain_to_remove.start, + offset, + deletions=[deletion for deletion in domain_to_remove.deletions if deletion < offset], + insertions=[insertion for insertion in domain_to_remove.insertions if insertion[0] < offset], + ) + domain_right: Domain = Domain( + helix, + forward, + offset, + domain_to_remove.end, + deletions=[deletion for deletion in domain_to_remove.deletions if deletion >= offset], + insertions=[insertion for insertion in domain_to_remove.insertions if insertion[0] >= offset], + ) # "before" and "after" mean in the 5' --> 3' direction, i.e., if a reverse domain: # <--------] @@ -8727,27 +8745,54 @@ def add_nick(self, helix: int, offset: int, forward: bool, new_color: bool = Tru domains_before = domains_before + cast(list[Domain | Loopout | Extension], [domain_to_add_before]) # noqa domains_after = cast(list[Domain | Loopout | Extension], [domain_to_add_after]) + domains_after # noqa + # number of bases 5' of the nick; internal modifications are indexed into the strand's DNA + # sequence, so this is where their indices get split (linear case) or rotated (circular case) + num_bases_before = sum(domain.dna_length() for domain in domains_before) + if strand.circular: # if strand is circular, we modify its domains in place domains = domains_after + domains_before strand.set_domains(domains) - # DNA sequence was rotated, so re-assign it + # The strand's 5' end moved from the start of the old first domain to just 3' of the nick, + # i.e., the base that was at index num_bases_before is now at index 0. Rotate the DNA + # sequence and the internal modification indices to follow. if seq_before_whole is not None and seq_after_whole is not None: - seq = seq_before_whole + seq_after_whole + seq = seq_after_whole + seq_before_whole strand.set_dna_sequence(seq) + dna_length = strand.dna_length() + strand.modifications_int = { + (idx - num_bases_before) % dna_length: mod for idx, mod in strand.modifications_int.items() + } + strand.set_linear() else: # if strand is not circular, we delete it and create two new strands - self.strands.remove(strand) + # 5' modification stays with the strand keeping the 5' end, 3' modification with the other, + # and each internal modification goes to whichever side of the nick its base is on + mods_int_before = { + idx: mod for idx, mod in strand.modifications_int.items() if idx < num_bases_before + } + mods_int_after = { + idx - num_bases_before: mod + for idx, mod in strand.modifications_int.items() + if idx >= num_bases_before + } + + # a name/label identifies a single strand, so it cannot be given to both new strands; it goes + # to the one keeping the 5' end, matching ligate and add_half_crossover strand_before: Strand = Strand( domains=domains_before, dna_sequence=seq_before_whole, color=strand.color, vendor_fields=strand.vendor_fields, + modification_5p=strand.modification_5p, + modifications_int=mods_int_before, + name=strand.name, + label=strand.label, ) color_after = next(self.color_cycler) if new_color else strand.color @@ -8755,8 +8800,12 @@ def add_nick(self, helix: int, offset: int, forward: bool, new_color: bool = Tru domains=domains_after, dna_sequence=seq_after_whole, color=color_after, + modification_3p=strand.modification_3p, + modifications_int=mods_int_after, ) + # both new strands constructed successfully, so it is now safe to swap them in + self.strands.remove(strand) # TODO: put in same position as original strand self.strands.extend([strand_before, strand_after]) @@ -8886,26 +8935,56 @@ def ligate(self, helix: int, offset: int, forward: bool) -> None: strand = strand_left assert strand.first_bound_domain() is dom_5p assert strand.last_bound_domain() is dom_3p + + # A circular strand cannot have a 5'/3' modification. Check before mutating the strand + # below, so that a strand that cannot legally be made circular is left untouched. + if strand.modification_5p is not None: + raise StrandError(strand, "cannot have a 5' modification on a circular strand") + if strand.modification_3p is not None: + raise StrandError(strand, "cannot have a 3' modification on a circular strand") + + old_dna_sequence = strand.dna_sequence + # after joining, the strand starts at the 5' end of dom_3p rather than that of dom_5p + num_bases_rotated = dom_3p.dna_length() + strand.domains[0] = dom_new # set first domain equal to new joined domain strand.domains.pop() # remove last domain strand.set_circular() for domain in strand.domains: domain._parent_strand = strand + # dom_new is a fresh Domain with no DNA sequence, so the sequence must be re-assigned; + # the last num_bases_rotated bases (those of dom_3p) are now the first ones. The internal + # modification indices are rotated by the same amount to stay on the same bases. + if old_dna_sequence is not None: + strand.set_dna_sequence( + old_dna_sequence[-num_bases_rotated:] + old_dna_sequence[:-num_bases_rotated] + ) + dna_length = strand.dna_length() + strand.modifications_int = { + (idx + num_bases_rotated) % dna_length: mod for idx, mod in strand.modifications_int.items() + } + else: # join strands old_strand_3p_dna_sequence = strand_3p.dna_sequence old_strand_5p_dna_sequence = strand_5p.dna_sequence + # number of bases strand_3p contributes to the front of the joined strand; must be read + # before its domains are merged with strand_5p's below + num_bases_strand_3p = strand_3p.dna_length() strand_3p.domains.pop() strand_3p.domains.append(dom_new) strand_3p.domains.extend(strand_5p.domains[1:]) strand_3p.is_scaffold = strand_left.is_scaffold or strand_right.is_scaffold - if strand_5p.modification_3p is not None: - strand_3p.set_modification_3p(strand_5p.modification_3p) + + # strand_3p supplies the 5' part of the joined strand, so it keeps its own 5' modification + # and its internal modification indices, but its 3' modification (if any) now sits at the + # ligated nick in the middle of the strand, so it is replaced by strand_5p's. + strand_3p.modification_3p = strand_5p.modification_3p for idx, mod in strand_5p.modifications_int.items(): - new_idx = idx + strand_3p.dna_length() - strand_3p.set_modification_internal(new_idx, mod) + strand_3p.modifications_int[idx + num_bases_strand_3p] = mod + if old_strand_3p_dna_sequence is not None and old_strand_5p_dna_sequence is not None: strand_3p.set_dna_sequence(old_strand_3p_dna_sequence + old_strand_5p_dna_sequence) if strand_5p.is_scaffold and not strand_3p.is_scaffold and strand_5p.color is not None: @@ -9022,12 +9101,28 @@ def add_half_crossover( raise IllegalDesignError( "cannot add crossover between two strands if one has a DNA sequence and the other does not" ) + + # strand_first supplies the 5' part of the new strand and strand_last the 3' part, so the new + # strand takes its 5' modification from strand_first and its 3' modification from strand_last. + # (strand_first's 3' modification and strand_last's 5' modification are at the crossover, in the + # middle of the new strand, so they cannot be kept.) strand_first's internal modifications keep + # their indices, and strand_last's shift up by the number of bases now preceding them. + new_modifications_int = dict(strand_first.modifications_int) + for idx, mod in strand_last.modifications_int.items(): + new_modifications_int[idx + strand_first.dna_length()] = mod + new_strand: Strand = Strand( domains=new_domains, color=strand_first.color, dna_sequence=new_dna, vendor_fields=strand_first.vendor_fields, is_scaffold=strand1.is_scaffold or strand2.is_scaffold, + modification_5p=strand_first.modification_5p, + modification_3p=strand_last.modification_3p, + modifications_int=new_modifications_int, + # the new strand takes the name/label of the strand supplying its 5' end + name=strand_first.name, + label=strand_first.label, ) # put new strand in place where strand_first was diff --git a/tests/scadnano_tests.py b/tests/scadnano_tests.py index 13beafe..bd92ca5 100644 --- a/tests/scadnano_tests.py +++ b/tests/scadnano_tests.py @@ -3958,6 +3958,535 @@ def test_nick_on_extension(self) -> None: self.assertIn(expected_strand2, design.strands) +def base_addresses(strand: sc.Strand) -> list[tuple[int, bool, int]]: + """ + Physical address (helix, forward, offset) of each base of `strand`, indexed by the base's position + in :data:`Strand.dna_sequence`, i.e., ``base_addresses(strand)[i]`` is the address of the base at + index `i` of the strand's DNA sequence. Accounts for deletions (the offset has no base) and + insertions (the offset has more than one base). Assumes `strand` has no loopouts/extensions. + + This is an independent way of working out where each base physically sits, so a test can assert that + a strand edit left an internal modification on the same base it started on. + """ + addresses = [] + for domain in strand.domains: + offsets = range(domain.start, domain.end) if domain.forward else range(domain.end - 1, domain.start - 1, -1) + insertion_lengths = dict(domain.insertions) + for offset in offsets: + if offset in domain.deletions: + continue + num_bases = 1 + insertion_lengths.get(offset, 0) + addresses.extend([(domain.helix, domain.forward, offset)] * num_bases) + return addresses + + +class TestModificationsOnStrandEdits(unittest.TestCase): + """ + Tests that add_nick(), ligate(), add_half_crossover(), and add_full_crossover() preserve + :any:`Modification`'s on the :any:`Strand`'s they edit. + + https://github.com/UC-Davis-molecular-computing/scadnano-python-package/issues/321 + + When two strands are joined, with strand A supplying the 5' part of the joined strand and strand B + the 3' part, the joined strand's 5' modification is A's, its 3' modification is B's, and its internal + modifications are A's at unchanged indices together with B's shifted up by A's DNA length. + Splitting a strand is the reverse. In all cases an internal modification must stay attached to the + same physical base. + """ + + def assert_mods_on_same_bases( + self, strand: sc.Strand, expected: dict[tuple[int, bool, int], sc.ModificationInternal] + ) -> None: + """Assert that the internal mods of `strand` sit on the bases at the given physical addresses.""" + addresses = base_addresses(strand) + actual = {addresses[idx]: mod_int for idx, mod_int in strand.modifications_int.items()} + self.assertDictEqual(expected, actual) + + def setUp(self) -> None: + r""" + two_strand_design: + 0 8 16 + AACCGGTT AACCGGTT + 0 [------- -------> + 1 <------- -------] + TTGGCCAA TTGGCCAA + """ + self.design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + self.design.draw_strand(0, 0).move(16) + self.design.draw_strand(1, 16).move(-16) + self.forward_strand = self.design.strands[0] + self.reverse_strand = self.design.strands[1] + self.forward_strand.set_dna_sequence("AACCGGTT" * 2) + self.reverse_strand.set_dna_sequence("TTGGCCAA" * 2) + + ############################################################################## + # add_half_crossover + + def test_add_half_crossover__preserves_modifications(self) -> None: + r""" + Joins the two strands at offset 15, so the forward strand supplies the 5' half of the joined + strand and the reverse strand the 3' half: + + 0 8 16 + 0 [------- --------\ + 1 <------- --------/ + """ + self.forward_strand.set_modification_5p(mod.biotin_5p) + self.forward_strand.set_modification_3p(mod.cy5_3p) # at the junction; cannot survive + self.forward_strand.set_modification_internal(4, mod.cy3_int) + self.reverse_strand.set_modification_5p(mod.cy3_5p) # at the junction; cannot survive + self.reverse_strand.set_modification_3p(mod.biotin_3p) + self.reverse_strand.set_modification_internal(2, mod.cy5_int) + + self.design.add_half_crossover(helix=0, helix2=1, offset=15, forward=True) + + self.assertEqual(1, len(self.design.strands)) + strand = self.design.strands[0] + self.assertEqual(mod.biotin_5p, strand.modification_5p) + self.assertEqual(mod.biotin_3p, strand.modification_3p) + # reverse strand's internal mod at index 2 shifts up by the forward strand's length (16) + self.assertDictEqual({4: mod.cy3_int, 18: mod.cy5_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(0, True, 4): mod.cy3_int, (1, False, 13): mod.cy5_int}) + + def test_add_half_crossover__preserves_modifications_other_strand_first(self) -> None: + r""" + Joins at offset 0, so now the *reverse* strand supplies the 5' half of the joined strand. + This exercises the other branch of add_half_crossover, where strand_first is strand2. + + 0 8 16 + 0 \------- -------> + 1 /------- -------] + """ + self.reverse_strand.set_modification_5p(mod.biotin_5p) + self.reverse_strand.set_modification_internal(2, mod.cy5_int) + self.forward_strand.set_modification_3p(mod.biotin_3p) + self.forward_strand.set_modification_internal(4, mod.cy3_int) + + self.design.add_half_crossover(helix=0, helix2=1, offset=0, forward=True) + + self.assertEqual(1, len(self.design.strands)) + strand = self.design.strands[0] + self.assertEqual(mod.biotin_5p, strand.modification_5p) + self.assertEqual(mod.biotin_3p, strand.modification_3p) + self.assertDictEqual({2: mod.cy5_int, 20: mod.cy3_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(1, False, 13): mod.cy5_int, (0, True, 4): mod.cy3_int}) + + def test_add_half_crossover__to_itself_with_terminal_modification_raises(self) -> None: + """A circular strand cannot carry a 5'/3' modification, so this must raise rather than + silently drop it.""" + design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + design.draw_strand(0, 0).move(8).cross(1).move(-8) + design.strands[0].set_modification_5p(mod.biotin_5p) + with self.assertRaises(sc.StrandError): + design.add_half_crossover(helix=0, helix2=1, offset=0, forward=True) + + ############################################################################## + # add_full_crossover + + def test_add_full_crossover__preserves_modifications(self) -> None: + r""" + add_full_crossover nicks both strands at offset 8 and then adds two half crossovers, turning the + two 16-base strands into two different 16-base strands, each half on helix 0 and half on helix 1: + + 0 8 16 + 0 [-------\ /------> + 1 <-------/ \------] + + Every internal modification must end up on the same physical base it started on, even though the + strand it belongs to, and its index within that strand, both change. + """ + self.forward_strand.set_modification_5p(mod.biotin_5p) + self.forward_strand.set_modification_internal(4, mod.cy3_int) # helix 0 offset 4 + self.forward_strand.set_modification_internal(12, mod.fluorescein_int) # helix 0 offset 12 + self.reverse_strand.set_modification_3p(mod.biotin_3p) + self.reverse_strand.set_modification_internal(2, mod.cy5_int) # helix 1 offset 13 + + self.design.add_full_crossover(helix=0, helix2=1, offset=8, forward=True) + + self.assertEqual(2, len(self.design.strands)) + + # this strand runs from helix 0 offset 0 across to helix 1 and ends at helix 1 offset 0, + # so it keeps the forward strand's 5' mod and the reverse strand's 3' mod + strand_left = strand_matching(self.design.strands, 0, True, 0, 8) + self.assertEqual(mod.biotin_5p, strand_left.modification_5p) + self.assertEqual(mod.biotin_3p, strand_left.modification_3p) + self.assertDictEqual({4: mod.cy3_int}, strand_left.modifications_int) + self.assert_mods_on_same_bases(strand_left, {(0, True, 4): mod.cy3_int}) + + # this strand runs from helix 1 offset 15 across to helix 0 and ends at helix 0 offset 15; + # it has no terminal mods, and picks up an internal mod from each of the original strands + strand_right = strand_matching(self.design.strands, 1, False, 8, 16) + self.assertIsNone(strand_right.modification_5p) + self.assertIsNone(strand_right.modification_3p) + self.assertDictEqual({2: mod.cy5_int, 12: mod.fluorescein_int}, strand_right.modifications_int) + self.assert_mods_on_same_bases( + strand_right, {(1, False, 13): mod.cy5_int, (0, True, 12): mod.fluorescein_int} + ) + + ############################################################################## + # ligate + + def test_ligate__preserves_modifications(self) -> None: + r""" + 0 8 16 + 0 [------> [------> + becomes + 0 [------- -------> + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(8) + design.draw_strand(0, 8).move(8) + strand_left, strand_right = design.strands + strand_left.set_dna_sequence("AACCGGTT") + strand_right.set_dna_sequence("TTGGCCAA") + strand_left.set_modification_5p(mod.biotin_5p) + strand_left.set_modification_3p(mod.cy5_3p) # at the nick; cannot survive + strand_left.set_modification_internal(2, mod.cy3_int) + strand_right.set_modification_5p(mod.cy3_5p) # at the nick; cannot survive + strand_right.set_modification_3p(mod.biotin_3p) + strand_right.set_modification_internal(3, mod.cy5_int) + + design.ligate(helix=0, offset=8, forward=True) + + self.assertEqual(1, len(design.strands)) + strand = design.strands[0] + self.assertEqual(mod.biotin_5p, strand.modification_5p) + self.assertEqual(mod.biotin_3p, strand.modification_3p) + self.assertDictEqual({2: mod.cy3_int, 11: mod.cy5_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(0, True, 2): mod.cy3_int, (0, True, 11): mod.cy5_int}) + + def test_ligate__preserves_modifications_reverse(self) -> None: + r""" + 0 8 16 + 0 <------] <------] + becomes + 0 <------- -------] + The 5' end of the joined strand is on the *right*, so the right-hand strand supplies the 5' part. + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 16).move(-8) + design.draw_strand(0, 8).move(-8) + strand_right, strand_left = design.strands + strand_right.set_dna_sequence("AACCGGTT") + strand_left.set_dna_sequence("TTGGCCAA") + strand_right.set_modification_5p(mod.biotin_5p) + strand_right.set_modification_internal(2, mod.cy3_int) + strand_left.set_modification_3p(mod.biotin_3p) + strand_left.set_modification_internal(3, mod.cy5_int) + + design.ligate(helix=0, offset=8, forward=False) + + self.assertEqual(1, len(design.strands)) + strand = design.strands[0] + self.assertEqual(mod.biotin_5p, strand.modification_5p) + self.assertEqual(mod.biotin_3p, strand.modification_3p) + self.assertDictEqual({2: mod.cy3_int, 11: mod.cy5_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(0, False, 13): mod.cy3_int, (0, False, 4): mod.cy5_int}) + + def test_ligate__stale_3p_modification_at_nick_is_removed(self) -> None: + """ + The 3' modification of the strand on the 5' side of the nick ends up in the middle of the + joined strand, so it must not be left behind as the joined strand's 3' modification. + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(8) + design.draw_strand(0, 8).move(8) + strand_left, strand_right = design.strands + strand_left.set_modification_3p(mod.cy5_3p) # at the nick; must not survive + + design.ligate(helix=0, offset=8, forward=True) + + self.assertEqual(1, len(design.strands)) + self.assertIsNone(design.strands[0].modification_3p) + + def test_ligate__circular_rotates_dna_and_internal_modifications(self) -> None: + r""" + Ligating a strand to itself makes it circular, which rotates the strand's 5' end from the 5' end + of its first domain to the 5' end of its last domain. The DNA sequence and the internal + modification indices must follow. + + 0 4 8 + 0 /---][---\ + 1 \--------/ + """ + design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + design.draw_strand(0, 4).move(4).cross(1).move(-8).cross(0).move(4) + strand = design.strands[0] + strand.set_dna_sequence("AAAA" + "CCCCGGGG" + "TTTT") + strand.set_modification_internal(0, mod.cy3_int) # first 'A', at helix 0 offset 4 + strand.set_modification_internal(12, mod.cy5_int) # first 'T', at helix 0 offset 0 + + design.ligate(helix=0, offset=4, forward=True) + + self.assertEqual(1, len(design.strands)) + strand = design.strands[0] + self.assertTrue(strand.circular) + # the joined domain (0, True, 0, 8) is now the strand's first domain, so the sequence starts + # with the 'TTTT' that used to be at the 3' end + self.assertEqual("TTTT" + "AAAA" + "CCCCGGGG", strand.dna_sequence) + self.assertDictEqual({4: mod.cy3_int, 0: mod.cy5_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(0, True, 4): mod.cy3_int, (0, True, 0): mod.cy5_int}) + + def test_ligate__to_itself_with_terminal_modification_raises_and_design_unchanged(self) -> None: + """A circular strand cannot carry a 5'/3' modification, so this must raise; and because the + check fails, the design must be left exactly as it was.""" + design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + design.draw_strand(0, 4).move(4).cross(1).move(-8).cross(0).move(4) + strand = design.strands[0] + strand.set_dna_sequence("AAAA" + "CCCCGGGG" + "TTTT") + strand.set_modification_5p(mod.biotin_5p) + + with self.assertRaises(sc.StrandError): + design.ligate(helix=0, offset=4, forward=True) + + self.assertFalse(strand.circular) + self.assertEqual(3, len(strand.domains)) + self.assertEqual("AAAA" + "CCCCGGGG" + "TTTT", strand.dna_sequence) + + ############################################################################## + # add_nick + + def test_add_nick__preserves_modifications(self) -> None: + r""" + 0 8 16 + 0 [------- -------> + becomes + 0 [------> [------> + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(16) + strand = design.strands[0] + strand.set_dna_sequence("AACCGGTT" * 2) + strand.set_modification_5p(mod.biotin_5p) + strand.set_modification_3p(mod.biotin_3p) + strand.set_modification_internal(2, mod.cy3_int) + strand.set_modification_internal(11, mod.cy5_int) + + design.add_nick(helix=0, offset=8, forward=True) + + self.assertEqual(2, len(design.strands)) + strand_before = strand_matching(design.strands, 0, True, 0, 8) + strand_after = strand_matching(design.strands, 0, True, 8, 16) + + self.assertEqual(mod.biotin_5p, strand_before.modification_5p) + self.assertIsNone(strand_before.modification_3p) + self.assertDictEqual({2: mod.cy3_int}, strand_before.modifications_int) + self.assert_mods_on_same_bases(strand_before, {(0, True, 2): mod.cy3_int}) + + self.assertIsNone(strand_after.modification_5p) + self.assertEqual(mod.biotin_3p, strand_after.modification_3p) + # index 11 on the original strand is index 11 - 8 = 3 on the 3' strand + self.assertDictEqual({3: mod.cy5_int}, strand_after.modifications_int) + self.assert_mods_on_same_bases(strand_after, {(0, True, 11): mod.cy5_int}) + + def test_add_nick__preserves_modifications_reverse(self) -> None: + r""" + 0 8 16 + 0 <------- -------] + becomes + 0 <------] <------] + The 5' end is on the right, so the nick's "before" strand is the right-hand one. + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 16).move(-16) + strand = design.strands[0] + strand.set_dna_sequence("AACCGGTT" * 2) + strand.set_modification_5p(mod.biotin_5p) + strand.set_modification_3p(mod.biotin_3p) + strand.set_modification_internal(2, mod.cy3_int) + strand.set_modification_internal(11, mod.cy5_int) + + design.add_nick(helix=0, offset=8, forward=False) + + self.assertEqual(2, len(design.strands)) + strand_before = strand_matching(design.strands, 0, False, 8, 16) + strand_after = strand_matching(design.strands, 0, False, 0, 8) + + self.assertEqual(mod.biotin_5p, strand_before.modification_5p) + self.assertDictEqual({2: mod.cy3_int}, strand_before.modifications_int) + self.assert_mods_on_same_bases(strand_before, {(0, False, 13): mod.cy3_int}) + + self.assertEqual(mod.biotin_3p, strand_after.modification_3p) + self.assertDictEqual({3: mod.cy5_int}, strand_after.modifications_int) + self.assert_mods_on_same_bases(strand_after, {(0, False, 4): mod.cy5_int}) + + def test_add_nick__circular_rotates_dna_and_internal_modifications(self) -> None: + r""" + Nicking a circular strand makes it linear, rotating its domains so that the strand now starts + just 3' of the nick. The DNA sequence and internal modification indices must follow. + + 0 8 + 0 /-------\ + 1 \---]<--/ + """ + design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + design.draw_strand(0, 0).move(8).cross(1).move(-8).as_circular() + strand = design.strands[0] + strand.set_dna_sequence("AAAACCCC" + "GGGGTTTT") + strand.set_modification_internal(0, mod.cy3_int) # first 'A', at helix 0 offset 0 + strand.set_modification_internal(8, mod.cy5_int) # first 'G', at helix 1 offset 7 + + design.add_nick(helix=1, offset=4, forward=False) + + self.assertEqual(1, len(design.strands)) + strand = design.strands[0] + self.assertFalse(strand.circular) + # new domain order is (1,False,0,4), (0,True,0,8), (1,False,4,8), so the sequence now starts + # with the 'TTTT' that the reverse domain reads out over offsets 3..0 + self.assertEqual("TTTT" + "AAAACCCC" + "GGGG", strand.dna_sequence) + self.assertDictEqual({4: mod.cy3_int, 12: mod.cy5_int}, strand.modifications_int) + self.assert_mods_on_same_bases(strand, {(0, True, 0): mod.cy3_int, (1, False, 7): mod.cy5_int}) + + ############################################################################## + # deletions/insertions, which change Domain.dna_length() and so shift internal mod indices + + def test_add_nick__carries_deletions_and_insertions_to_the_correct_side(self) -> None: + r""" + A deletion or insertion changes how many bases a domain has, so it changes the DNA index of every + base 3' of it. add_nick must give each deletion/insertion to the piece whose offsets contain it, + or the internal modifications (and the DNA sequence) end up on the wrong bases. + + 0 X 8 I 16 X = deletion at offset 4 + 0 [-----------------------> I = insertion of 2 at offset 12 + """ + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(16).with_deletions(4).with_insertions((12, 2)) + strand = design.strands[0] + # 16 offsets - 1 deletion + 2 inserted bases = 17 bases + self.assertEqual(17, strand.dna_length()) + strand.set_dna_sequence("AACCGGTTAACCGGTTA") + strand.set_modification_internal(2, mod.cy3_int) # 5' of the nick, at helix 0 offset 2 + strand.set_modification_internal(10, mod.cy5_int) # 3' of the nick, at helix 0 offset 11 + + design.add_nick(helix=0, offset=8, forward=True) + + strand_before = strand_matching(design.strands, 0, True, 0, 8) + strand_after = strand_matching(design.strands, 0, True, 8, 16) + # the deletion at offset 4 belongs to the 5' piece, the insertion at offset 12 to the 3' piece + self.assertEqual([4], strand_before.domains[0].deletions) + self.assertEqual([], strand_before.domains[0].insertions) + self.assertEqual([], strand_after.domains[0].deletions) + self.assertEqual([(12, 2)], strand_after.domains[0].insertions) + self.assertEqual(7, strand_before.dna_length()) # 8 offsets - 1 deletion + self.assertEqual(10, strand_after.dna_length()) # 8 offsets + 2 inserted bases + # and the DNA is intact: the two pieces still spell the original sequence + self.assertEqual("AACCGGT", strand_before.dna_sequence) + self.assertEqual("TAACCGGTTA", strand_after.dna_sequence) + + self.assertDictEqual({2: mod.cy3_int}, strand_before.modifications_int) + self.assert_mods_on_same_bases(strand_before, {(0, True, 2): mod.cy3_int}) + # DNA index 10 of the original strand is index 10 - 7 = 3 on the 3' piece + self.assertDictEqual({3: mod.cy5_int}, strand_after.modifications_int) + self.assert_mods_on_same_bases(strand_after, {(0, True, 11): mod.cy5_int}) + + def test_add_nick_then_ligate__round_trips_with_deletions_and_insertions(self) -> None: + """Nicking and then ligating at the same place must restore the original strand exactly.""" + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(16).with_deletions(4).with_insertions((12, 2)) + strand = design.strands[0] + strand.set_dna_sequence("AACCGGTTAACCGGTTA") + strand.set_modification_5p(mod.biotin_5p) + strand.set_modification_3p(mod.biotin_3p) + strand.set_modification_internal(2, mod.cy3_int) + strand.set_modification_internal(10, mod.cy5_int) + + design.add_nick(helix=0, offset=8, forward=True) + design.ligate(helix=0, offset=8, forward=True) + + self.assertEqual(1, len(design.strands)) + strand = design.strands[0] + self.assertEqual([4], strand.domains[0].deletions) + self.assertEqual([(12, 2)], strand.domains[0].insertions) + self.assertEqual("AACCGGTTAACCGGTTA", strand.dna_sequence) + self.assertEqual(mod.biotin_5p, strand.modification_5p) + self.assertEqual(mod.biotin_3p, strand.modification_3p) + self.assertDictEqual({2: mod.cy3_int, 10: mod.cy5_int}, strand.modifications_int) + + +class TestNamesOnStrandEdits(unittest.TestCase): + """ + Tests that add_nick(), ligate(), add_half_crossover(), and add_full_crossover() preserve + :data:`Strand.name` and :data:`Strand.label`. + + https://github.com/UC-Davis-molecular-computing/scadnano-python-package/issues/320 + + The rule is that the name/label of the 5' :any:`Strand` wins: when two strands are joined, the joined + strand takes the name of the one supplying its 5' end, and when a strand is split, the piece keeping + the 5' end keeps the name. (:py:meth:`Design.ligate` already worked this way, since it joins by + mutating the 5' strand in place.) + """ + + def setUp(self) -> None: + r""" + 0 8 16 + 0 [------- -------> "forward" + 1 <------- -------] "reverse" + """ + self.design = sc.Design(helices=[sc.Helix(max_offset=32) for _ in range(2)]) + self.design.draw_strand(0, 0).move(16).with_name("forward").with_label("forward_label") + self.design.draw_strand(1, 16).move(-16).with_name("reverse").with_label("reverse_label") + + def test_add_half_crossover__preserves_name_and_label_of_5p_strand(self) -> None: + """Joining at offset 15 makes the forward strand the 5' half, so its name/label win.""" + self.design.add_half_crossover(helix=0, helix2=1, offset=15, forward=True) + self.assertEqual(1, len(self.design.strands)) + self.assertEqual("forward", self.design.strands[0].name) + self.assertEqual("forward_label", self.design.strands[0].label) + + def test_add_half_crossover__preserves_name_and_label_of_5p_strand_other_strand_first(self) -> None: + """Joining at offset 0 makes the reverse strand the 5' half, so now *its* name/label win. + This exercises the other branch of add_half_crossover, where strand_first is strand2.""" + self.design.add_half_crossover(helix=0, helix2=1, offset=0, forward=True) + self.assertEqual(1, len(self.design.strands)) + self.assertEqual("reverse", self.design.strands[0].name) + self.assertEqual("reverse_label", self.design.strands[0].label) + + def test_add_nick__keeps_name_and_label_on_5p_strand(self) -> None: + """The piece keeping the 5' end keeps the name; the other piece gets no name (a name identifies + one strand, so it cannot be given to both pieces).""" + self.design.add_nick(helix=0, offset=8, forward=True) + + strand_before = strand_matching(self.design.strands, 0, True, 0, 8) + strand_after = strand_matching(self.design.strands, 0, True, 8, 16) + self.assertEqual("forward", strand_before.name) + self.assertEqual("forward_label", strand_before.label) + self.assertIsNone(strand_after.name) + self.assertIsNone(strand_after.label) + + def test_add_nick__keeps_name_on_5p_strand_reverse(self) -> None: + """For a reverse domain the 5' end is on the right, so the right-hand piece keeps the name.""" + self.design.add_nick(helix=1, offset=8, forward=False) + + strand_before = strand_matching(self.design.strands, 1, False, 8, 16) + strand_after = strand_matching(self.design.strands, 1, False, 0, 8) + self.assertEqual("reverse", strand_before.name) + self.assertIsNone(strand_after.name) + + def test_add_full_crossover__preserves_names(self) -> None: + """add_full_crossover nicks and then joins, so both original names must survive, each on the + strand that inherits the corresponding 5' end.""" + self.design.add_full_crossover(helix=0, helix2=1, offset=8, forward=True) + + self.assertEqual(2, len(self.design.strands)) + # starts at helix 0 offset 0, which was the forward strand's 5' end + self.assertEqual("forward", strand_matching(self.design.strands, 0, True, 0, 8).name) + # starts at helix 1 offset 15, which was the reverse strand's 5' end + self.assertEqual("reverse", strand_matching(self.design.strands, 1, False, 8, 16).name) + + def test_ligate__preserves_name_and_label_of_5p_strand(self) -> None: + """Regression guard: ligate already keeps the 5' strand's name, since it joins by mutating that + strand in place.""" + design = sc.Design(helices=[sc.Helix(max_offset=32)]) + design.draw_strand(0, 0).move(8).with_name("five_prime").with_label("five_prime_label") + design.draw_strand(0, 8).move(8).with_name("three_prime") + + design.ligate(helix=0, offset=8, forward=True) + + self.assertEqual(1, len(design.strands)) + self.assertEqual("five_prime", design.strands[0].name) + self.assertEqual("five_prime_label", design.strands[0].label) + + class TestAutocalculatedData(unittest.TestCase): def test_helix_min_max_offsets_illegal_explicitly_specified(self) -> None: helices = [sc.Helix(min_offset=5, max_offset=5)] From bf3f9bb7f5de85ba96978c65462a287cdd4e24c3 Mon Sep 17 00:00:00 2001 From: David Doty Date: Sun, 12 Jul 2026 09:28:18 -0700 Subject: [PATCH 2/5] added Python 3.14 to CI unit test action --- .github/workflows/run_unit_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run_unit_tests.yml b/.github/workflows/run_unit_tests.yml index 51f8071..4055275 100644 --- a/.github/workflows/run_unit_tests.yml +++ b/.github/workflows/run_unit_tests.yml @@ -11,7 +11,7 @@ jobs: shell: bash -el {0} # Use conda shell strategy: matrix: - python-version: [ "3.10", "3.11", "3.12", "3.13" ] + python-version: [ "3.10", "3.11", "3.12", "3.13", "3.14" ] steps: - uses: actions/checkout@v6 From ad7f9ecd8623dabd6dd754dc3963c42d94d2a307 Mon Sep 17 00:00:00 2001 From: David Doty Date: Sun, 12 Jul 2026 09:43:57 -0700 Subject: [PATCH 3/5] fixed unit tests to not mix Helix's that have an explicit idx with those that don't --- tests/scadnano_tests.py | 58 ++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/tests/scadnano_tests.py b/tests/scadnano_tests.py index bd92ca5..1cafa07 100644 --- a/tests/scadnano_tests.py +++ b/tests/scadnano_tests.py @@ -4587,19 +4587,21 @@ def setUp(self) -> None: e = "east" s = "south" w = "west" + # when helices are given as a list, either all of them specify idx or none of them do, so since + # the last two need an explicit idx, all of them state theirs explicitly helices = [ - sc.Helix(max_offset=20, group=n, grid_position=(1, 1)), # 0 - sc.Helix(max_offset=21, group=n, grid_position=(0, 1)), # 1 - sc.Helix(max_offset=19, group=n, grid_position=(0, 2)), # 2 - sc.Helix(max_offset=18, group=n, grid_position=(1, 2)), # 3 - sc.Helix(max_offset=17, group=n, grid_position=(2, 2)), # 4 - sc.Helix(max_offset=16, group=n, grid_position=(2, 1)), # 5 - sc.Helix(max_offset=24, group=s), # 6 - sc.Helix(max_offset=25, group=s), # 7 - sc.Helix(max_offset=26, group=w, position=sc.Position3D(x=0, y=0, z=0)), # 8 - sc.Helix(max_offset=27, group=w, position=sc.Position3D(x=0, y=2.5, z=0)), # 9 - sc.Helix(idx=13, max_offset=22, group=e), # 13 - sc.Helix(idx=15, max_offset=23, group=e), # 15 + sc.Helix(idx=0, max_offset=20, group=n, grid_position=(1, 1)), + sc.Helix(idx=1, max_offset=21, group=n, grid_position=(0, 1)), + sc.Helix(idx=2, max_offset=19, group=n, grid_position=(0, 2)), + sc.Helix(idx=3, max_offset=18, group=n, grid_position=(1, 2)), + sc.Helix(idx=4, max_offset=17, group=n, grid_position=(2, 2)), + sc.Helix(idx=5, max_offset=16, group=n, grid_position=(2, 1)), + sc.Helix(idx=6, max_offset=24, group=s), + sc.Helix(idx=7, max_offset=25, group=s), + sc.Helix(idx=8, max_offset=26, group=w, position=sc.Position3D(x=0, y=0, z=0)), + sc.Helix(idx=9, max_offset=27, group=w, position=sc.Position3D(x=0, y=2.5, z=0)), + sc.Helix(idx=13, max_offset=22, group=e), + sc.Helix(idx=15, max_offset=23, group=e), ] group_north = sc.HelixGroup(position=sc.Position3D(x=0, y=-200, z=0), grid=sc.honeycomb) group_south = sc.HelixGroup(position=sc.Position3D(x=0, y=70, z=0), helices_view_order=[7, 6], grid=sc.square) @@ -4617,6 +4619,38 @@ def setUp(self) -> None: self.s = s self.w = w + def test_helices_mixing_specified_and_unspecified_idx_raises_error(self) -> None: + """ + When helices are given as a list, either all specify idx or none do. This is the mix that setUp + used to have (some helices with no idx, followed by helices with an explicit idx), which went + undetected: the check recorded only the *first* un-indexed helix, and once it had, the branch + that records an explicitly-indexed helix could no longer run, so the two were never seen + together and no error was raised. + + The explicit indices here (13 and 15) are chosen so they cannot collide with the indices 0 and 1 + that the first two helices would be auto-assigned. Otherwise the duplicate-index check would + raise instead, and this test would pass without ever exercising the mixing check. + """ + helices = [ + sc.Helix(max_offset=20), + sc.Helix(max_offset=21), + sc.Helix(idx=13, max_offset=22), + sc.Helix(idx=15, max_offset=23), + ] + with self.assertRaisesRegex(sc.IllegalDesignError, "either all helices must have idx"): + sc.Design(helices=helices, strands=[], grid=sc.square) + + def test_helices_mixing_unspecified_and_specified_idx_raises_error(self) -> None: + """Same, but with the explicitly-indexed helices first, so that neither ordering can regress.""" + helices = [ + sc.Helix(idx=13, max_offset=22), + sc.Helix(idx=15, max_offset=23), + sc.Helix(max_offset=20), + sc.Helix(max_offset=21), + ] + with self.assertRaisesRegex(sc.IllegalDesignError, "either all helices must have idx"): + sc.Design(helices=helices, strands=[], grid=sc.square) + def test_helix_groups(self) -> None: self._asserts_for_fixture(self.design) From b1be729473f6783e7e2f002e48715df92f9123e1 Mon Sep 17 00:00:00 2001 From: David Doty Date: Sun, 12 Jul 2026 10:57:00 -0700 Subject: [PATCH 4/5] CI unit tests: install dependencies with pip instead of conda The conda `defaults` channel needs a separately compiled build of each package for each Python version, and it lags behind new Python releases, which is what broke the Python 3.14 job: docutils 0.18 has no 3.14 build there, so the environment could not be solved and the tests never ran. openpyxl and tabulate both publish pure-Python universal wheels, so pip installs them on any Python version, including future ones. Also drop docutils, which the unit tests never import. It is only needed to build the docs, and there it comes in automatically as a dependency of Sphinx (docs-check.yml and readthedocs both just install sphinx), so it never needed to be named here. The pin was stale besides: Sphinx now requires docutils >= 0.21. Add fail-fast: false so that one Python version failing no longer cancels the jobs for the others. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/run_unit_tests.yml | 49 +++++++++++++--------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/.github/workflows/run_unit_tests.yml b/.github/workflows/run_unit_tests.yml index 4055275..e0dde84 100644 --- a/.github/workflows/run_unit_tests.yml +++ b/.github/workflows/run_unit_tests.yml @@ -1,26 +1,23 @@ -name: Run Unit Tests - -on: pull_request - -jobs: - build: - - runs-on: ubuntu-latest - defaults: - run: - shell: bash -el {0} # Use conda shell - strategy: - matrix: - python-version: [ "3.10", "3.11", "3.12", "3.13", "3.14" ] - - steps: - - uses: actions/checkout@v6 - - name: Setup conda and python - uses: conda-incubator/setup-miniconda@v3 - with: - auto-activate: true - python-version: ${{ matrix.python-version }} - - name: Install openpyxl,tabulate,docutils with conda - run: conda install openpyxl=3.1 tabulate=0.9 docutils=0.18 - - name: Test with unittest - run: python -m unittest -v tests/scadnano_tests.py +name: Run Unit Tests + +on: pull_request + +jobs: + build: + + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: [ "3.10", "3.11", "3.12", "3.13", "3.14" ] + + steps: + - uses: actions/checkout@v6 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Install openpyxl,tabulate with pip + run: pip install openpyxl tabulate + - name: Test with unittest + run: python -m unittest -v tests/scadnano_tests.py From a77d90d23acd7e81a19501ec817c9b18839897bc Mon Sep 17 00:00:00 2001 From: David Doty Date: Sun, 12 Jul 2026 10:58:43 -0700 Subject: [PATCH 5/5] CI unit tests: upgrade checkout and setup-python actions checkout v6 -> v7 and setup-python v5 -> v6, the current majors. Both run on Node 24; the older majors run on Node runtimes GitHub is retiring. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/run_unit_tests.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run_unit_tests.yml b/.github/workflows/run_unit_tests.yml index e0dde84..9ff9832 100644 --- a/.github/workflows/run_unit_tests.yml +++ b/.github/workflows/run_unit_tests.yml @@ -12,9 +12,11 @@ jobs: python-version: [ "3.10", "3.11", "3.12", "3.13", "3.14" ] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 + # the runner image ships only one Python on PATH; this is what actually selects the version + # named by the matrix (all of 3.10-3.14 are in the runner's tool cache, so it is a fast lookup) - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + uses: actions/setup-python@v6 with: python-version: ${{ matrix.python-version }} - name: Install openpyxl,tabulate with pip