diff --git a/python/CHANGELOG.rst b/python/CHANGELOG.rst index 7455ad5910..a3c8a47da5 100644 --- a/python/CHANGELOG.rst +++ b/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`` diff --git a/python/tests/test_highlevel.py b/python/tests/test_highlevel.py index d83f053f0a..fe9fd498df 100644 --- a/python/tests/test_highlevel.py +++ b/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", diff --git a/python/tests/test_tables.py b/python/tests/test_tables.py index 63e85dead5..6284ab10ba 100644 --- a/python/tests/test_tables.py +++ b/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( diff --git a/python/tskit/tables.py b/python/tskit/tables.py index 57a33113e9..0a2517641c 100644 --- a/python/tskit/tables.py +++ b/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, diff --git a/python/tskit/trees.py b/python/tskit/trees.py index b1e5fa8fe0..ed9bd82f8a 100644 --- a/python/tskit/trees.py +++ b/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,