tskit icon indicating copy to clipboard operation
tskit copied to clipboard

Tree.sites iterator isn't reversable

Open jeromekelleher opened this issue 3 years ago • 2 comments
trafficstars

It would be good to make the Tree.sites method reversable so that we can get at the sites backwards when running through the trees in reverse.

You can always just do reversed(list(tree.sites())) at the moment, though.

While making the change, we should update the low-level API to return a numpy array of site IDs.

It's then a question of whether it's worth keeping the iterator behaviour or if we should just return a list of site objects. I guess there is potentially some performance creating Site objects lazily on demand, but most of the time we'll just be creating them one after another.

An option would be to create a SiteRange object which looks a bit like the TreeIterator but also supports __getitem__ and creates site objects on demand.

Any thoughts?

jeromekelleher avatar Feb 03 '22 15:02 jeromekelleher

Keeping things idiomatic by returning an iterator seems like a good idea to me. We could have a second function that returns an array if anyone wants to deal with the entire array at once?

molpopgen avatar Feb 03 '22 15:02 molpopgen

Yeah, an array of site_ids would probably be quite handy. Probably the SiteRange implementation is the way to go then, it's easy enough to implement.

jeromekelleher avatar Feb 03 '22 16:02 jeromekelleher