diff --git a/scadnano/scadnano.py b/scadnano/scadnano.py index 689ef06..b7616bd 100644 --- a/scadnano/scadnano.py +++ b/scadnano/scadnano.py @@ -105,6 +105,9 @@ default_scadnano_file_extension = "sc" """Default filename extension when writing a scadnano file.""" +excel_sheet_name_max_length = 31 +# maximum length of an Excel sheet name, used to check if plate name exceeds this length + VStrands = dict[int, dict[str, Any]] @@ -8104,6 +8107,7 @@ def write_idt_plate_excel_file( use_default_plates: bool = True, warn_using_default_plates: bool = True, plate_type: PlateType = PlateType.wells96, + raise_exception_if_plate_name_long: bool = True, export_non_modified_strand_version: bool = False, ) -> None: """ @@ -8161,12 +8165,29 @@ def write_idt_plate_excel_file( if the `use_default_plates` parameter is ``True``. Ignored if `use_default_plates` is ``False``, because in that case the wells are explicitly set by the user, who is free to use coordinates for either plate type. + :param raise_exception_if_plate_name_long: + Excel will not allow a sheet name longer than 31 characters. If this parameter is True (the default), then + an exception is raised if any :data:`VendorFields.plate` field is longer than 31 characters for any strand. + Set this parameter to False to allow the program to automatically truncate any plate name longer than + 31 characters. + If set to False and any plate name exceeds 31 characters, a warning is printed for any plate name that + is truncated, reminding the user to manually change the plate name on IDT's website. + Since plate names are chosen by this method (``plate1``, ``plate2``, ...) rather than by the user + when `use_default_plates` is ``True``, they can never be too long in that case, so setting this + parameter to False requires also setting `use_default_plates` to ``False``. + Two plates cannot share one sheet, so if two plate names are identical in their first + 31 characters, an exception is raised regardless of the value of this parameter. :param export_non_modified_strand_version: For any :any:`Strand` with a :any:`Modification`, also export a version of the :any:`Strand` without any modifications. The name for this :any:`Strand` is the original name with '_nomods' appended to it. """ + if (not raise_exception_if_plate_name_long) and use_default_plates: + raise ValueError( + "If raise_exception_if_plate_name_long is False then use_default_plates must also be False" + ) + strands_to_export = self._idt_strands_to_export( key=key, warn_duplicate_name=warn_duplicate_name, @@ -8180,7 +8201,12 @@ def write_idt_plate_excel_file( raise ValueError( "parameters use_default_plates and only_strands_with_vendor_fields cannot both be False" ) - self._write_plates_assuming_explicit_plates_in_each_strand(directory, filename, strands_to_export) + self._write_plates_assuming_explicit_plates_in_each_strand( + directory, + filename, + strands_to_export, + raise_exception_if_plate_name_long=raise_exception_if_plate_name_long, + ) else: self._write_plates_default( directory=directory, @@ -8190,10 +8216,53 @@ def write_idt_plate_excel_file( warn_using_default_plates=warn_using_default_plates, ) + @staticmethod + def _msg_plate_name_too_long(plate_name: str, err: bool) -> str: + msg_prefix = ( + f'The plate name "{plate_name}" is longer than {excel_sheet_name_max_length} characters, ' + f"which is the maximum allowed by Excel." + ) + if err: + msg = ( + msg_prefix + + """ + Please shorten the plate name in the VendorFields.plate field of the strands in this plate, + or set the parameter raise_exception_if_plate_name_long=False in the call to Design.write_idt_plate_excel_file. + """ + ) + else: + msg = f"""\ +WARNING: {msg_prefix} +The plate name will be truncated to {excel_sheet_name_max_length} characters in the Excel file: +"{plate_name[:excel_sheet_name_max_length]}" +Please manually change the plate name on IDT's website if you want to retain the original name +{plate_name} + """ + return msg + + @staticmethod + def _msg_plate_names_share_sheet_name(plate_name1: str, plate_name2: str, sheet_name: str) -> str: + return f"""\ +The plate names +"{plate_name1}" +"{plate_name2}" +would both be written to a sheet named +"{sheet_name}" +since Excel allows at most {excel_sheet_name_max_length} characters in a sheet name, +and two plates cannot share one sheet. +Please shorten the plate names in the VendorFields.plate field of the strands on these plates so that +they differ within their first {excel_sheet_name_max_length} characters. +Setting raise_exception_if_plate_name_long=False does not avoid this error. + """ + def _write_plates_assuming_explicit_plates_in_each_strand( - self, directory: str, filename: str | None, strands_to_export: list[Strand] + self, + directory: str, + filename: str | None, + strands_to_export: list[Strand], + raise_exception_if_plate_name_long: bool, ) -> None: - plates = list( + plate_names = list( { strand.vendor_fields.plate for strand in strands_to_export @@ -8201,7 +8270,7 @@ def _write_plates_assuming_explicit_plates_in_each_strand( if strand.vendor_fields.plate is not None } ) - if len(plates) == 0: + if len(plate_names) == 0: raise ValueError( "Cannot write a a plate file since no plate data exists in any Strands " "in the design.\n" @@ -8209,15 +8278,47 @@ def _write_plates_assuming_explicit_plates_in_each_strand( "Design.write_idt_plate_excel_file\nif you don't want to enter plate " "and well positions for each Strand you wish to write to the Excel file." ) - plates.sort() + plate_names.sort() + + # Excel cannot handle a sheet name longer than excel_sheet_name_max_length, so each plate name is + # mapped to the name of the sheet it will be written to, which is the plate name itself unless it + # is too long and we were asked to truncate rather than raise. The plate name itself is still what + # identifies the strands on the plate, so the two must be kept separate. + sheet_name_of_plate: dict[str, str] = {} + plate_of_sheet_name: dict[str, str] = {} + truncated_plate_names: list[str] = [] + for plate_name in plate_names: + if len(plate_name) <= excel_sheet_name_max_length: + sheet_name = plate_name + elif raise_exception_if_plate_name_long: + msg = self._msg_plate_name_too_long(plate_name, err=True) + raise ValueError(msg) + else: + sheet_name = plate_name[:excel_sheet_name_max_length] + truncated_plate_names.append(plate_name) + + # Distinct plate names can collapse to the same sheet name once truncated, and two plates + # cannot share one sheet, so this is an error even though we were asked not to raise an + # exception for a long plate name. + if sheet_name in plate_of_sheet_name: + msg = self._msg_plate_names_share_sheet_name(plate_of_sheet_name[sheet_name], plate_name, sheet_name) + raise ValueError(msg) + plate_of_sheet_name[sheet_name] = plate_name + sheet_name_of_plate[plate_name] = sheet_name + + # warn only once the names of all sheets are known to be usable, so that no warning is printed + # about a plate name that one of the exceptions above then rejects the whole file for + for plate_name in truncated_plate_names: + print(self._msg_plate_name_too_long(plate_name, err=False)) + filename_plate, workbook = self._setup_excel_file(directory, filename) - for plate in plates: - worksheet = self._add_new_excel_plate_sheet(plate, workbook) + for plate_name in plate_names: + worksheet = self._add_new_excel_plate_sheet(sheet_name_of_plate[plate_name], workbook) strands_in_plate = [ strand for strand in strands_to_export - if strand.vendor_fields is not None and strand.vendor_fields.plate == plate + if strand.vendor_fields is not None and strand.vendor_fields.plate == plate_name ] strands_in_plate.sort(key=lambda s: (int(s.vendor_fields.well[1:]), s.vendor_fields.well[0])) # type: ignore @@ -8258,14 +8359,14 @@ def _write_plates_default( directory: str, filename: str | None, strands: list[Strand], - plate_type: PlateType = PlateType.wells96, - warn_using_default_plates: bool = True, + plate_type: PlateType, + warn_using_default_plates: bool, ) -> None: plate_coord = PlateCoordinate(plate_type=plate_type) - plate = 1 + plate_idx = 1 excel_row = 1 filename_plate, workbook = self._setup_excel_file(directory, filename) - worksheet = self._add_new_excel_plate_sheet(f"plate{plate}", workbook) + worksheet = self._add_new_excel_plate_sheet(f"plate{plate_idx}", workbook) num_strands_per_plate = plate_type.num_wells_per_plate() num_plates_needed = len(strands) // num_strands_per_plate @@ -8313,10 +8414,10 @@ def _write_plates_default( else: plate_coord.advance() - if plate != plate_coord.plate(): + if plate_idx != plate_coord.plate(): workbook.save(filename_plate) - plate = plate_coord.plate() - worksheet = self._add_new_excel_plate_sheet(f"plate{plate}", workbook) + plate_idx = plate_coord.plate() + worksheet = self._add_new_excel_plate_sheet(f"plate{plate_idx}", workbook) excel_row = 1 else: excel_row += 1 @@ -8782,13 +8883,9 @@ def add_nick(self, helix: int, offset: int, forward: bool, new_color: bool = Tru # 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_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 + 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 @@ -8966,9 +9063,7 @@ def ligate(self, helix: int, offset: int, forward: bool) -> None: # 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] - ) + 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() diff --git a/tests/scadnano_tests.py b/tests/scadnano_tests.py index 6d75dd4..78dcd0b 100644 --- a/tests/scadnano_tests.py +++ b/tests/scadnano_tests.py @@ -5,6 +5,8 @@ import tempfile import unittest import re +import io +import contextlib import json import math from typing import Iterable, Union, Dict, Any @@ -1443,6 +1445,340 @@ def test_export_dna_sequences_loopout(self) -> None: self.assertEqual("strand,AAAAATTTAAAAA,25nm,STD", contents) +class TestWriteIdtPlateExcelFilePlateNames(unittest.TestCase): + """ + Tests the interaction of the parameters `raise_exception_if_plate_name_long` and `use_default_plates` + of :py:meth:`scadnano.Design.write_idt_plate_excel_file`. + + Each plate becomes one worksheet, named after the plate, and Excel silently truncates a worksheet name + longer than :py:data:`scadnano.excel_sheet_name_max_length` characters, so a long + :py:data:`scadnano.VendorFields.plate` name would otherwise be corrupted without the user noticing. + """ + + strand_len = 10 + dna_sequence = "T" * strand_len + header_row = ["Well Position", "Name", "Sequence"] + + def setUp(self) -> None: + self.max_len = sc.excel_sheet_name_max_length + tmpdir = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + self.addCleanup(tmpdir.cleanup) + self.filename = os.path.join(tmpdir.name, "plates.xlsx") + + @classmethod + def design_with_plates(cls, plates_and_wells: list[tuple[str, str]]) -> sc.Design: + """ + Design with one strand per (plate, well) pair given, the strand at index i being named ``si``. + """ + max_offset = cls.strand_len * len(plates_and_wells) + design = sc.Design(helices=[sc.Helix(max_offset=max_offset)], strands=[], grid=sc.square) + for idx, (plate, well) in enumerate(plates_and_wells): + design.draw_strand(0, cls.strand_len * idx).move(cls.strand_len).with_name(f"s{idx}").with_sequence( + cls.dna_sequence + ).with_vendor_fields(plate=plate, well=well) + return design + + def load_sheets(self) -> list[tuple[str, list[list[Any]]]]: + """ + Contents of the written file as a list of (worksheet name, rows) pairs, in worksheet order. + """ + book = openpyxl.load_workbook(filename=self.filename) + return [(sheet.title, [[cell.value for cell in row] for row in sheet.iter_rows()]) for sheet in book.worksheets] + + def sheet_names(self) -> list[str]: + return [name for name, _ in self.load_sheets()] + + def strand_row(self, well: str, name: str) -> list[Any]: + return [well, name, self.dna_sequence] + + def test_excel_sheet_name_max_length(self) -> None: + # the tests below build plate names relative to this constant; 31 is the limit Excel itself imposes + self.assertEqual(31, sc.excel_sheet_name_max_length) + + ################################################# + # raise_exception_if_plate_name_long=False requires use_default_plates=False + + def test_raise_false_with_default_plates_true_rejected(self) -> None: + design = self.design_with_plates([("plate_a", "A1")]) + with self.assertRaises(ValueError) as cm: + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=True, + raise_exception_if_plate_name_long=False, + ) + msg = str(cm.exception) + self.assertIn("raise_exception_if_plate_name_long", msg) + self.assertIn("use_default_plates", msg) + self.assertFalse(os.path.exists(self.filename)) + + def test_raise_false_with_default_plates_omitted_rejected(self) -> None: + # use_default_plates defaults to True, so omitting it is the same as passing True + design = self.design_with_plates([("plate_a", "A1")]) + with self.assertRaises(ValueError): + design.write_idt_plate_excel_file(filename=self.filename, raise_exception_if_plate_name_long=False) + self.assertFalse(os.path.exists(self.filename)) + + ################################################# + # use_default_plates=True: plates are named plate1, plate2, ..., so no plate name can be too long + + def test_default_plates_ignore_long_plate_names(self) -> None: + long_name = "L" * (self.max_len + 9) + design = self.design_with_plates([(long_name, "A1"), (long_name, "A2")]) + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=True, + warn_using_default_plates=False, + raise_exception_if_plate_name_long=True, + ) + # the explicit plate names are discarded in favor of default plate/well addressing + self.assertEqual( + [("plate1", [self.header_row, self.strand_row("A1", "s0"), self.strand_row("B1", "s1")])], + self.load_sheets(), + ) + + def test_default_plates_with_only_strands_with_vendor_fields_false(self) -> None: + # this is the default combination of parameters, so it must be accepted + design = self.design_with_plates([("plate_a", "A1"), ("plate_a", "A2")]) + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=True, + only_strands_with_vendor_fields=False, + warn_using_default_plates=False, + raise_exception_if_plate_name_long=True, + ) + self.assertEqual(["plate1"], self.sheet_names()) + + ################################################# + # use_default_plates=False, raise_exception_if_plate_name_long=True + + def test_explicit_plates_short_names_unchanged(self) -> None: + design = self.design_with_plates([("plate_c", "A1"), ("plate_a", "A1"), ("plate_b", "A1"), ("plate_a", "A2")]) + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + self.assertEqual( + [ + ("plate_a", [self.header_row, self.strand_row("A1", "s1"), self.strand_row("A2", "s3")]), + ("plate_b", [self.header_row, self.strand_row("A1", "s2")]), + ("plate_c", [self.header_row, self.strand_row("A1", "s0")]), + ], + self.load_sheets(), + ) + + def test_explicit_plates_long_name_raises(self) -> None: + long_name = "targ_1995_larg_frag_newM13_shrt_int_adps" # the example from issue #348 + self.assertGreater(len(long_name), self.max_len) + design = self.design_with_plates([(long_name, "A1"), ("plate_a", "A1")]) + with self.assertRaises(ValueError) as cm: + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + msg = str(cm.exception) + self.assertIn(long_name, msg) + self.assertIn(str(self.max_len), msg) + self.assertIn("raise_exception_if_plate_name_long=False", msg) + # nothing is written, rather than a file missing the offending plate + self.assertFalse(os.path.exists(self.filename)) + + def test_explicit_plates_long_name_raises_by_default(self) -> None: + # raise_exception_if_plate_name_long defaults to True + design = self.design_with_plates([("P" * (self.max_len + 1), "A1")]) + with self.assertRaises(ValueError): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + ) + self.assertFalse(os.path.exists(self.filename)) + + def test_explicit_plates_name_of_exactly_max_length_allowed(self) -> None: + name = "P" * self.max_len + design = self.design_with_plates([(name, "A1")]) + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + self.assertEqual([(name, [self.header_row, self.strand_row("A1", "s0")])], self.load_sheets()) + + def test_explicit_plates_name_one_over_max_length_raises(self) -> None: + name = "P" * (self.max_len + 1) + design = self.design_with_plates([(name, "A1")]) + with self.assertRaises(ValueError): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + + def test_long_plate_name_only_on_unexported_scaffold_does_not_raise(self) -> None: + # the length check applies to the strands actually exported, not to every strand in the design + long_name = "S" * (self.max_len + 5) + design = sc.Design(helices=[sc.Helix(max_offset=2 * self.strand_len)], strands=[], grid=sc.square) + design.draw_strand(0, 0).move(self.strand_len).with_name("scaf").with_sequence( + self.dna_sequence + ).with_vendor_fields(plate=long_name, well="A1").as_scaffold() + design.draw_strand(0, self.strand_len).move(self.strand_len).with_name("s0").with_sequence( + self.dna_sequence + ).with_vendor_fields(plate="plate_a", well="A1") + + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + self.assertEqual([("plate_a", [self.header_row, self.strand_row("A1", "s0")])], self.load_sheets()) + + # but exporting the scaffold brings its long plate name into the scope of the check + with self.assertRaises(ValueError): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + export_scaffold=True, + raise_exception_if_plate_name_long=True, + ) + + ################################################# + # use_default_plates=False, raise_exception_if_plate_name_long=False + + def test_explicit_plates_long_name_truncated(self) -> None: + long_name = "targ_1995_larg_frag_newM13_shrt_int_adps" # the example from issue #348 + truncated_name = long_name[: self.max_len] + design = self.design_with_plates([(long_name, "A1"), (long_name, "A2"), ("plate_a", "A1")]) + + printed = io.StringIO() + with contextlib.redirect_stdout(printed): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=False, + ) + + # worksheets are ordered by the original plate names, but named by the truncated ones + self.assertEqual( + [ + ("plate_a", [self.header_row, self.strand_row("A1", "s2")]), + # the strands on the long-named plate must survive the truncation of its name + (truncated_name, [self.header_row, self.strand_row("A1", "s0"), self.strand_row("A2", "s1")]), + ], + self.load_sheets(), + ) + for name in self.sheet_names(): + self.assertLessEqual(len(name), self.max_len) + + # the warning names the plate both as it was and as it will appear in the file + warning = printed.getvalue() + self.assertIn(long_name, warning) + self.assertIn(truncated_name, warning) + self.assertIn(str(self.max_len), warning) + + def test_explicit_plates_short_names_not_truncated_and_no_warning(self) -> None: + design = self.design_with_plates([("plate_a", "A1"), ("P" * self.max_len, "A1")]) + printed = io.StringIO() + with contextlib.redirect_stdout(printed): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=False, + ) + self.assertEqual(["P" * self.max_len, "plate_a"], self.sheet_names()) + self.assertEqual("", printed.getvalue()) + + ################################################# + # two plate names that are identical in their first excel_sheet_name_max_length characters would name + # the same sheet, which is an error however raise_exception_if_plate_name_long is set + + def test_explicit_plates_two_long_names_colliding_after_truncation_raises(self) -> None: + shared_prefix = "targ_1995_larg_frag_newM13_shrt" + self.assertEqual(self.max_len, len(shared_prefix)) + long_name1 = shared_prefix + "_int_adps" + long_name2 = shared_prefix + "_ext_adps" + design = self.design_with_plates([(long_name1, "A1"), (long_name2, "A1"), ("plate_a", "A1")]) + + printed = io.StringIO() + with contextlib.redirect_stdout(printed): + with self.assertRaises(ValueError) as cm: + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=False, + ) + msg = str(cm.exception) + self.assertIn(long_name1, msg) + self.assertIn(long_name2, msg) + self.assertIn(shared_prefix, msg) + self.assertFalse(os.path.exists(self.filename)) + # no truncation warning is printed for a file that is then rejected anyway + self.assertEqual("", printed.getvalue()) + + def test_explicit_plates_long_name_colliding_with_short_name_raises(self) -> None: + # the shorter name is not truncated at all, but the longer one truncates onto it + short_name = "targ_1995_larg_frag_newM13_shrt" + self.assertEqual(self.max_len, len(short_name)) + long_name = short_name + "_int_adps" + design = self.design_with_plates([(short_name, "A1"), (long_name, "A1")]) + + with self.assertRaises(ValueError) as cm: + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=False, + ) + msg = str(cm.exception) + self.assertIn(short_name, msg) + self.assertIn(long_name, msg) + self.assertFalse(os.path.exists(self.filename)) + + def test_explicit_plates_colliding_names_raise_for_being_long_when_raise_true(self) -> None: + # with raise_exception_if_plate_name_long=True the names are rejected for their length, before + # there is any truncation for them to collide under + shared_prefix = "P" * self.max_len + design = self.design_with_plates([(shared_prefix + "1", "A1"), (shared_prefix + "2", "A1")]) + with self.assertRaises(ValueError) as cm: + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=True, + ) + self.assertIn("raise_exception_if_plate_name_long=False", str(cm.exception)) + + def test_explicit_plates_long_names_differing_before_truncation_do_not_collide(self) -> None: + # the names differ in their last character before the cutoff, so truncating them keeps them distinct + name1 = "P" * (self.max_len - 1) + "1" + "X" * 9 + name2 = "P" * (self.max_len - 1) + "2" + "X" * 9 + design = self.design_with_plates([(name1, "A1"), (name2, "A1")]) + with contextlib.redirect_stdout(io.StringIO()): + design.write_idt_plate_excel_file( + filename=self.filename, + use_default_plates=False, + only_strands_with_vendor_fields=True, + raise_exception_if_plate_name_long=False, + ) + self.assertEqual([name1[: self.max_len], name2[: self.max_len]], self.sheet_names()) + self.assertEqual( + [ + (name1[: self.max_len], [self.header_row, self.strand_row("A1", "s0")]), + (name2[: self.max_len], [self.header_row, self.strand_row("A1", "s1")]), + ], + self.load_sheets(), + ) + + class TestExportCadnanoV2(unittest.TestCase): """ Tests the export feature to cadnano v2 (see misc/cadnano-format-specs/v2.txt).