Skip to content

Add TreeSequence.coiterate(other, **kwargs) method #1021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeromekelleher opened this issue Nov 20, 2020 · 6 comments · Fixed by #1022
Closed

Add TreeSequence.coiterate(other, **kwargs) method #1021

jeromekelleher opened this issue Nov 20, 2020 · 6 comments · Fixed by #1022
Assignees
Labels
Python API Issue is about the Python API
Milestone

Comments

@jeromekelleher
Copy link
Member

As discussed in #815, this is a useful operation and should be part of the public API.

The implementation is here, which I think is probably fine. We can leave the door open to coiterating along more than two tree sequences via *args, but we don't need to worry about doing it for now.

We just need a handful of tests to make sure that (a) this does what we expect on a simple example; (b) the trees are returned in the correct order (i.e., tree1.tree_sequence is ts1) and so on; (c) we test some cases like one ts has one tree and the other has 3, etc; (d) test error conditions are handled correctly.

I think we should get this in for 0.3.3

@benjeffery @hyanwong, would one of you mind picking this up please?

@jeromekelleher jeromekelleher added the Python API Issue is about the Python API label Nov 20, 2020
@jeromekelleher jeromekelleher added this to the Python 0.3.3 milestone Nov 20, 2020
@hyanwong
Copy link
Member

I can pick this one up if you like. Something to do while waiting for rescomp to come back up. Should be easy I think.

@jeromekelleher
Copy link
Member Author

Great, thanks @hyanwong.

@hyanwong
Copy link
Member

Shall I put it somewhere like tskit/util.py or have it as a method on a ts in trees.py?

@jeromekelleher
Copy link
Member Author

I think it's a method of TreeSequence: TreeSequence.coiterate(self, other, **kwargs)

@hyanwong
Copy link
Member

Which reminds me, trees.py is getting pretty long now. I wonder if we should split it into tree.py and tree_sequence.py? If you think this is a good idea @jeromekelleher I'll open it as an issue.

@jeromekelleher
Copy link
Member Author

Feel free to open an issue to gather opinions.

hyanwong added a commit to hyanwong/tskit that referenced this issue Nov 20, 2020
hyanwong added a commit to hyanwong/tskit that referenced this issue Nov 20, 2020
hyanwong added a commit to hyanwong/tskit that referenced this issue Nov 21, 2020
hyanwong added a commit to hyanwong/tskit that referenced this issue Nov 23, 2020
hyanwong added a commit to hyanwong/tskit that referenced this issue Nov 23, 2020
AdminBot-tskit pushed a commit to hyanwong/tskit that referenced this issue Nov 23, 2020
@mergify mergify bot closed this as completed in #1022 Nov 23, 2020
jeromekelleher pushed a commit to jeromekelleher/tskit that referenced this issue Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python API Issue is about the Python API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants