Tskit: Rename map_ancestors to map_to_ancestors?

[jeromekelleher]

With the map_mutations method about to be added in #125, I wonder if the map_ancestors function is now a little confusingly named. Maybe map_to_ancestors would be a little more descriptive? Or perhaps trace_ancestors?

Don't worry @gtsambos, if we decide to change the name I can make the changes!

[gtsambos]

Either of those alternatives are fine by me! :)

[petrelharp]

Another idea: find_ancestors? When you write the paper, @gtsambos, are you going to want to call it "mapping"?

[jeromekelleher]

We're providing the ancestors as input though, so 'find' seems slightly off.

[gtsambos]

I'm not especially wedded to the word 'map'. I think 'find' is fine too, given that we don't a priori know which of/whether any of our provided ancestors are actually ancestral to the sample. Perhaps find_ancestral_relationships is more to the point, but a bit too long.

[jeromekelleher]

How about link_ancestors? We're creating a set of direct links from the samples to the ancestors, which we output via a set of edges.

[gtsambos]

I like link_ancestors!

[petrelharp]

What do you call it when you talk about it? Would you say "I have developed a method to link samples to their ancestors"?

I'd go for link_to_ancestors.

[petrelharp]

Although note that this function has other uses: if passed no ancestors, then it should find all relationships between samples (right?).

[gtsambos]

Would you say "I have developed a method to link samples to their ancestors"?

I think I would feel comfortable with that! 😉 'Extract' is the word I use most often, to convey the fact that the information I'm finding is already in the tree sequence object I'm analysing. But I think 'link' conveys that idea well enough.

Although note that this function has other uses: if passed no ancestors, then it should find all relationships between samples (right?).

Nope, the function will fail if it is passed no ancestors. If you want to find relationships between all samples, you should just pass samples and ancestors the same list of nodes.

[jeromekelleher]

OK, sounds like we have agreement. @gtsambos, can you make a call between link_ancestors and link_to_ancestors please? I'm happy with either.

[gtsambos]

Let's go link_ancestors because it's shorter.

[jeromekelleher]

We didn't end up implementing this before 0.2.1 got released, so we'll need to keep map_ancestors as a deprecated alias now. I think we can just 'undocument' it though and we don't need to formally deprecate it.

[gtsambos]

Hello, just returning to this because I'm about to code up the bit of slime that depends on this function, and it would be good to have the function name settled ( this will also make our write-up of it easier -- I'll send a separate email about this later in the week or early next)

Just want to confirm that link_ancestors is still okay for everyone? If so, I'll open a pull request for the change.

[jeromekelleher]

Yep, that'd be great thanks @gtsambos. See above the note about keeping the map_ancestors as an alias on the external Python API though.

[hyanwong]

@gtsambos - just looking at a few things to do for 0.2.3. Do you want me to raise a PR for link_ancestors, or do you have time to do it?

[gtsambos]

Hey @hyanwong, it depends how soon the release will be - I'm very tied up over the next 3-4 weeks, so if the release is imminent I'd really appreciate if you did it!

[jeromekelleher]

That's OK @gtsambos, we'll take care of it.

[hyanwong]

I'm happy to do this @jeromekelleher