Allow span to be set when generating trees #930
Conversation
|
📖 Docs for this PR can be previewed here |
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
Probably a useful thing to have. I'd be hesitant about invoking kwargs here though, we don't want to end up in matplotlib territory.
docs/python-api.rst
Outdated
| Combinatorics API | ||
| ***************** | ||
|
|
||
| The combinatorics API deals with tree topologies, allowing them to be enumerated and |
There was a problem hiding this comment.
Enumerate means "count". I would say "allowing them to be counted, listed and generated; see sec_..."
python/tskit/combinatorics.py
Outdated
|
|
||
|
|
||
| def all_trees(num_leaves): | ||
| def all_trees(num_leaves, **kwargs): |
There was a problem hiding this comment.
I'm not sure about this - why not just have span=None here? If we don't expect to have lots of parameters, then it's just making it hard for users to know where parameters come from. It's the same amount of typing, for now.
There was a problem hiding this comment.
I um-ed and ah-ed about putting it in. The argument for doing so is that we might want to add other features such as specifying branch length or node labels, which we would simply add to the underlying to_tsk_tree() method, and then get all the rest for free. But I could explicitly specify span for the time being (and repeat the docstring description for all the methods that call to_tsk_tree())
hyanwong
force-pushed
the
seq-len-for-ranks
branch
from
1120b8d to
e306584
Compare
|
OK, I think I've addressed all the comments now @jeromekelleher - just took the basic option of setting |
hyanwong
force-pushed
the
seq-len-for-ranks
branch
from
e306584 to
77489cb
Compare
|
Done, thanks @jeromekelleher |
|
Closing and reopening to force a travis rebuild |
AdminBot-tskit
force-pushed
the
seq-len-for-ranks
branch
from
77489cb to
20c0402
Compare
3 participants
Description
Extend the functionality of
Tree.unroot,tskit.all_treesetc to allow trees with spans other than 1. Passing**kwargsmeans that we can also extends this if (for example) we want to be able to set node/root times on the generated trees. Also document that a new ts is created for each new tree, and change the API section titles "Metadata" and "Combinatorics" so they aren't confused with the pages of those names.