update to tskit 1.0.0 #565
Conversation
petrelharp
force-pushed
the
tskit_v1b2
branch
from
9790618 to
84a12f8
Compare
|
Well this works, but how about let's just leave this open and I'll use it to merge in the official release when it comes. |
OK, the time has come for that! :-> |
omit integrity check in compute_mutation_parents
petrelharp
force-pushed
the
tskit_v1b2
branch
from
84a12f8 to
2f6ce23
Compare
| ret = tsk_table_collection_build_index(&output_tables, 0); | ||
| if (ret < 0) handle_error("tsk_table_collection_build_index", ret); | ||
| ret = tsk_table_collection_compute_mutation_parents(&output_tables, 0); | ||
| ret = tsk_table_collection_compute_mutation_parents(&output_tables, TSK_NO_CHECK_INTEGRITY); |
There was a problem hiding this comment.
What's the motivation for this change? Does this simply preserve the pre-existing behavior (because tskit's default behavior changed), or will this change SLiM's behavior in some way?
There was a problem hiding this comment.
Well, from the CHANGELOG to 1.2.0:
Add the TSK_NO_CHECK_INTEGRITY option to tsk_table_collection_compute_mutation_parents to skip the integrity checks that are normally run when computing mutation parents. This is useful for speeding up the computation of mutation parents when the tree sequence is certainly known to be valid. (:user:
benjeffery, :pr:3212).
So - this removes the "check integrity" check from this function, so in principle we might be less likely to catch an error; however, we have just checked integrity on the previous line in tsk_table_collection_build_index, so it's unnecessary.
| if (ret < 0) handle_error("tsk_table_collection_build_index", ret); | ||
|
|
||
| ret = tsk_table_collection_compute_mutation_parents(tables_copy, 0); | ||
| ret = tsk_table_collection_compute_mutation_parents(tables_copy, TSK_NO_CHECK_INTEGRITY); |
|
@petrelharp Had one question about this, see above. In addition to that: I'm expecting that tskit-dev/tskit#3306 will be fixed in the coming days, and to get that we'll need to merge tskit in again. Do you prefer that I merge tskit 1.0 here, and then merge whatever the following release is that has that addition as a separate PR? Or do you prefer to levae this PR unmerged for now, and then do a single merge later to get the addition? I'm fine either way; up to you since you're the one doing the work! :-> |
|
I guess I vote to merge this (pending checking about the question above), since it's cleaner. |
|
Question - I think - resolved; merge away? |
|
Done! Thanks! |
2 participants
Probably we should not merge this and just wait for tskit 1.0.0; but we'll see if there's any issues.