max_slim_mutation_id function

[mufernando]

Creates a function that returns the maximum SLiM mutation ID. Useful for adding more mutations with msprime.sim_mutations().

Solves #179

[petrelharp]

Looks good; we should also have a test that this actually lets us mutate a SLiM-produced tree sequence more and load it back in to SLiM?

[mufernando]

should be good to go! thanks @petrelharp

[petrelharp]

Hm - thinking a bit more, I think this should be next_slim_mutation_id( ), which returns the max plus one. That would mean that (a) we can say "just pass this value in" instead of "this value plus one"; and (b) it reutrns 0 in the case of no mutations, instead of -1.

[petrelharp]

Ah - and, we need to clarify (and, test) what this does for tree sequences whose derived states are not SLiM-produced. In the case of binary alleles (0/1), it will return 1 (or, 2 if we change to "next"); this is unavoidable but we should say in the docstring what it actually does so that people understand. If alleles are nucleotides, it'll do

>>> int("A")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: 'A'

so maybe we should try.. catch ValueError and give an informative error message?

[mufernando]

I changed it to give us the next mutation ID and introduced a try...except block to catch problems with different types of derived state.

[petrelharp]

I like it. A few suggested changes, and we need a test for the error.

[mufernando]

this should be good to go now!

[petrelharp]

Whoops - testing in more situations has turned up an issue: a derived or ancestral state of "" is valid (it means "no mutations"), and this produces an error. I think that right now the main implementation of this function would wrongly throw an error in this case, in fact.

[petrelharp]

Huh, and actually, this method should also iterate over sites.ancestral_state.

[mufernando]

@petrelharp and I talked about the ancestral state column in the Sites Table today and we don't see how that could be used here? SLiM doesn't use that column yet. So this should be ready to merge.