* Make sort_order private in SamBuilder to avoid changes/inconsistency

  with header
* Update poetry environment for newer python
* Insist sort_order is a SamOrder, elminate None option
* Allow writing with unknown sort order

Author: TedBrookings

fgpyo/sam/builder.py

@@ -106,7 +106,7 @@ def __init__(
         rg: Optional[Dict[str, str]] = None,
         extra_header: Optional[Dict[str, Any]] = None,
         seed: int = 42,
-        sort_order: Optional[SamOrder] = SamOrder.Coordinate,
+        sort_order: SamOrder = SamOrder.Coordinate,
     ) -> None:
         """Initializes a new SamBuilder for generating alignment records and SAM/BAM files.
 
@@ -119,21 +119,17 @@ def __init__(
             extra_header: a dictionary of extra values to add to the header, None otherwise.  See
                           `::class::~pysam.AlignmentHeader` for more details.
             seed: a seed value for random number/string generation
-            sort_order: optional sort order, if `None` reads will be output in the same order as
-                they were appended.
+            sort_order: Order to sort records when writing to file, or output of to_sorted_list()
         """
 
         self.r1_len: int = r1_len if r1_len is not None else self.DEFAULT_R1_LENGTH
         self.r2_len: int = r2_len if r2_len is not None else self.DEFAULT_R2_LENGTH
         self.base_quality: int = base_quality
         self.mapping_quality: int = mapping_quality
 
-        sort_order = SamOrder.Unsorted if sort_order is None else sort_order
-        assert sort_order in [SamOrder.Coordinate, SamOrder.QueryName, SamOrder.Unsorted], (
-            "`sort_order` for `SamBuilder` must be one of `Coordinate` `QueryName` or `Unsorted`."
-            + f" Found {sort_order}"
-        )
-        self.sort_order: SamOrder = sort_order
+        if not isinstance(sort_order, SamOrder):
+            raise ValueError(f"sort_order must be a SamOrder, got {type(sort_order)}")
+        self._sort_order = sort_order
 
         self._header: Dict[str, Any] = {
             "HD": {"VN": "1.5", "SO": sort_order.value},
@@ -580,7 +576,7 @@ def to_path(
 
         with NamedTemporaryFile(suffix=".bam", delete=True) as fp:
             file_handle: IO
-            if self.sort_order is SamOrder.Unsorted:
+            if self._sort_order in {SamOrder.Unsorted, SamOrder.Unknown}:
                 file_handle = path.open("w")
             else:
                 file_handle = fp.file
@@ -592,18 +588,17 @@ def to_path(
                     if pred(rec):
                         writer.write(rec)
 
-            default_samtools_opt_list = ["-o", str(path), fp.name]
+            samtools_sort_args = ["-o", str(path), fp.name]
 
             file_handle.close()
-            if self.sort_order == SamOrder.QueryName:
+            if self._sort_order == SamOrder.QueryName:
                 # Ignore type hints for now until we have wrappers to use here.
-                pysam.sort(*(["-n"] + default_samtools_opt_list))  # type: ignore
-            elif self.sort_order == SamOrder.Coordinate:
+                pysam.sort("-n", *samtools_sort_args)  # type: ignore
+            elif self._sort_order == SamOrder.Coordinate:
                 # Ignore type hints for now until we have wrappers to use here.
-                pysam.sort(*default_samtools_opt_list)  # type: ignore
                 if index:
-                    # Ignore type hints for now until we have wrappers to use here.
-                    pysam.index(str(path))  # type: ignore
+                    samtools_sort_args.insert(0, "--write-index")
+                pysam.sort(*samtools_sort_args)  # type: ignore
 
         return path
 

fgpyo/sam/tests/test_builder.py

@@ -272,9 +272,9 @@ def make_sort_order_builder(tmp_path: Path, sort_order: SamOrder) -> Path:
         (SamOrder.Coordinate, ["test2", "test3", "test4", "test1"]),
         (SamOrder.QueryName, ["test1", "test2", "test3", "test4"]),
         (SamOrder.Unsorted, ["test3", "test2", "test1", "test4"]),
-        (None, ["test3", "test2", "test1", "test4"]),
+        (SamOrder.Unknown, ["test3", "test2", "test1", "test4"]),
     ],
-    ids=["Coordinate sorting", "Query name sorting", "Unsorted output", "Unsorted output - None"],
+    ids=["Coordinate sorting", "Query name sorting", "Unsorted output", "Unknown sorting"],
 )
 def test_sort_types(
     tmp_path: Path, sort_order: Optional[SamOrder], expected_name_order: List[str]

poetry.lock

@@ -37,9 +37,13 @@ pysam = ">=0.22.0"
 docs = ["sphinx", "sphinx_rtd_theme"]
 
 [tool.poetry.dev-dependencies]
+setuptools = ">=68.0.0"
 pytest = ">=5.4.2"
 mypy = ">=0.770"
-flake8 = ">=3.8.1"
+flake8 = [
+	{ version = ">=3.8.1", python = "<3.12.0" },
+	{ version = ">=6.1.0", python = ">=3.12.0" },
+]
 black = ">=19.10b0"
 pytest-cov = ">=2.8.1"
 isort = ">=5.10.1"