While working through #360, it's become clear to me that there's some ugly inconsistencies in how we're doing things at the moment which needs to be resolved.
In the trees API that's in use, a node is always referred to by it's ID. So, we always say, e.g. tree.parent(0), returns the integer ID of node with ID 0. This is good because it's nice and efficient. We're not creating lots of objects for no good reason, and it's quite consistent.
However, things are getting more complicated with the generalised tree sequence API. Now, we do want to have container objects which represent the information we're interested in. For example, tree_sequence.node(0) with return the node object, which has time, population, etc attributes. Seems fine.
Things get a bit more complicated when we think about sites though. The sites iterator returns a sequence of Site objects. These have attributes position etc, which is fine. However, there's also a mutations attribute. Now, what should this be? Should it be:
a) A list of integer mutation IDs; or
b) A list of resolved mutation objects?
If (b), then should the mutation.node field be an integer node ID or a Node object? And the mutation.parent field?
We get a similar problem when we think about the variants iterator. This has a site field. Should that be a fully resolved and linked object chain (Site->[Mutation->Node]), or should it just be an ID?
Clearly it's more user-friendly to have fully resolved object chains, but I don't see how we can do this in a way that's consistent with the existing API. I'm not sure what to do here.
Any thoughts @petrelharp, @ashander??
I'd generally vote for keeping things simpler. As long as there's an easy way to extract the info you might want given the integer ID, then integer IDs are good.
Also, if everything is linked object chains, might there be circular references at some point? e.g. Site->[Mutation->Site]
I don't think that it has to be consistent in all cases. So, I think the current behavior is OK.
One maybe useful thought, though: It makes sense for Sites to carry lists of resolved Mutation objects because mutations are nested within sites, but not necessary for a mutation to carry a resolved copy of it's ("parent") mutation.
One maybe useful thought, though: It makes sense for Sites to carry lists of resolved Mutation objects because mutations are nested within sites, but not necessary for a mutation to carry a resolved copy of it's ("parent") mutation.
OK, this is actually quite a good and consistent thing to do. In the there is no 'mutations' field in the site table as an implicit relationship within the table structure. Therefore, filling in the resolved objects is a useful thing to do and doesn't clash with keeping IDs for all the other references. Phew!
Most helpful comment
OK, this is actually quite a good and consistent thing to do. In the there is no 'mutations' field in the site table as an implicit relationship within the table structure. Therefore, filling in the resolved objects is a useful thing to do and doesn't clash with keeping IDs for all the other references. Phew!