Make simplify use mostly keyword-only args

python/CHANGELOG.rst

@@ -2,6 +2,15 @@
 [X.X.X] - XXXX-XX-XX
 --------------------
 
+**Breaking changes**
+
+- Change several methods (``simplify()``, ``trees()``, ``Tree()``) so most parameters
+  are keyword only, not positional. This allows reordering of parameters, so
+  that deprecated parameters can be moved, and the parameter order in similar functions,
+  e.g. ``TableCollection.simplify`` and ``TreeSequence.simplify()`` can be made
+  consistent (:user:`hyanwong`, :issue:`374`, :issue:`846`, :pr:`851`)
+
+
 **Features**
 
 - Genomic intervals returned by python functions are now namedtuples, allowing ``.left``

python/tests/test_highlevel.py

@@ -789,7 +789,7 @@ def verify_tracked_samples(self, ts):
                 self.assertEqual(tree.get_num_tracked_samples(u), 0)
         samples = list(ts.samples())
         tracked_samples = samples[:2]
-        for tree in ts.trees(tracked_samples):
+        for tree in ts.trees(tracked_samples=tracked_samples):
             if len(tree.parent_dict) == 0:
                 # This is a crude way of checking if we have multiple roots.
                 # We'll need to fix this code up properly when we support multiple
@@ -809,6 +809,14 @@ def test_tracked_samples(self):
         for ts in get_example_tree_sequences():
             self.verify_tracked_samples(ts)
 
+    def test_tracked_samples_is_first_arg(self):
+        for ts in get_example_tree_sequences():
+            samples = list(ts.samples())[:2]
+            for a, b in zip(ts.trees(samples), ts.trees(tracked_samples=samples)):
+                self.assertEqual(
+                    a.get_num_tracked_samples(), b.get_num_tracked_samples()
+                )
+
     def test_deprecated_sample_aliases(self):
         for ts in get_example_tree_sequences():
             # Ensure that we get the same results from the various combinations
@@ -1381,6 +1389,26 @@ def test_sequence_iteration(self):
                     self.assertEqual(n.id, 0)
 
 
+class TestTreeSequenceMethodSignatures(unittest.TestCase):
+    ts = msprime.simulate(10, random_seed=1234)
+
+    def test_kwargs_only(self):
+        with self.assertRaisesRegex(TypeError, "argument"):
+            tskit.Tree(self.ts, [], True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.trees([], True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.haplotypes(True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.variants(True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.genotype_matrix(True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.simplify([], True)
+        with self.assertRaisesRegex(TypeError, "argument"):
+            self.ts.draw_svg("filename", True)
+
+
 class TestTreeSequenceMetadata(unittest.TestCase):
     metadata_tables = [
         "node",

python/tests/test_tables.py

@@ -2458,6 +2458,14 @@ def test_sequence_length_longer_than_edges(self):
         self.assertEqual(len(tree.parent_dict), 0)
 
 
+class TestTableCollectionMethodSignatures(unittest.TestCase):
+    tc = msprime.simulate(10, random_seed=1234).dump_tables()
+
+    def test_kwargs_only(self):
+        with self.assertRaises(TypeError):
+            self.tc.simplify([], True)
+
+
 class TestTableCollectionMetadata(unittest.TestCase):
 
     metadata_schema = metadata.MetadataSchema(

python/tskit/tables.py

@@ -2225,6 +2225,7 @@ def tree_sequence(self):
     def simplify(
         self,
         samples=None,
+        *,
         filter_zero_mutation_sites=None,  # Deprecated alias for filter_sites
         reduce_to_site_topology=False,
         filter_populations=True,

python/tskit/trees.py

@@ -660,6 +660,7 @@ def __init__(
         self,
         tree_sequence,
         tracked_samples=None,
+        *,
         sample_counts=None,
         sample_lists=False,
         root_threshold=1,
@@ -3652,6 +3653,7 @@ def last(self):
     def trees(
         self,
         tracked_samples=None,
+        *,
         sample_counts=None,
         sample_lists=False,
         tracked_leaves=None,
@@ -4440,6 +4442,7 @@ def to_macs(self):
     def simplify(
         self,
         samples=None,
+        *,
         filter_zero_mutation_sites=None,  # Deprecated alias for filter_sites
         map_nodes=False,
         reduce_to_site_topology=False,