Closed jorgeamaya closed 7 years ago
Hi @jorgeamaya
thanks for the Pull Request! It is nice work, but there are a couple of minor points I would like to have improved before we merge this in:
expand_mcmf
to FALSE, and make the summary statistics return in vector instead of a matrix in that case, so that we don't break existing code and pass the existing unit tests.Can you improve these two points? Then I'll resolve the minor merge conflict and merge this in.
Best, Paul
Hi @paulstaab
Sorry for the delay, quite a bit busy with my Thesis... I will start working on this now.
Hi @paulstaab
These commits should solve the backward compatibility problem. I ran the test and they seem to be OK. Jaatha will need some fixes too, I will work on that now. The test for the expaned MCMF is still pending, I'll work on that after fixing Jaatha.
I have checked Jaatha and no changes will be necessary to integrate the previous commit. Could you let me know if I have to change something else (I'm planning to use this version of the mcmf in my paper)?
I see the following test failures:
Failed -------------------------------------------------------------------------
1. Error: mcmf is correctly calculation for normal loci (@test-sumstat-mcmf.R#11)
not a matrix
1: expect_equal(calc_mcmf(seg_sites, 1:4, FALSE), 0.5) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:11
2: compare(object, expected, ...)
3: calc_mcmf(seg_sites, 1:4, FALSE)
2. Error: mcmf is correctly calculation for locus trios (@test-sumstat-mcmf.R#23)
argument "locus_length" is missing, with no default
1: expect_equal(calc_mcmf(seg_sites, 1:4), c(4/12)) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:23
2: compare(object, expected, ...)
3: calc_mcmf(seg_sites, 1:4)
3. Error: mcmf is correctly calculation for diplod loci (@test-sumstat-mcmf.R#44)
not a matrix
1: expect_equal(calc_mcmf(seg_sites, 1:2, FALSE, ploidy = 2), 0.75) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:44
2: compare(object, expected, ...)
3: calc_mcmf(seg_sites, 1:2, FALSE, ploidy = 2)
4. Error: mcmf is correctly calculation for trioplod loci (@test-sumstat-mcmf.R#58)
not a matrix
1: expect_equal(calc_mcmf(seg_sites, 1:2, FALSE, ploidy = 3), 0.75) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:58
2: compare(object, expected, ...)
3: calc_mcmf(seg_sites, 1:2, FALSE, ploidy = 3)
5. Error: initialzation of statistic works (@test-sumstat-mcmf.R#64) -----------
not compatible with requested type
1: expect_equal(stat$calculate(seg_sites, NULL, NULL, coal_model(4)), 0.5) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:64
2: compare(object, expected, ...)
3: stat$calculate(seg_sites, NULL, NULL, coal_model(4))
4: calc_mcmf(seg_sites, get_population_individuals(model, private$population, haploids = (ploidy ==
1)), get_locus_length_matrix(model), private$expand_mcmf, private$type_expand,
has_trios(model), ploidy) at /home/paul/coala/R/sumstat_mcmf.R:23
6. Error: mcmf statistics is correct for diploid models (@test-sumstat-mcmf.R#77)
not compatible with requested type
1: expect_equal(stat$calculate(seg_sites, NULL, NULL, model), 0.5) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:77
2: compare(object, expected, ...)
3: stat$calculate(seg_sites, NULL, NULL, model)
4: calc_mcmf(seg_sites, get_population_individuals(model, private$population, haploids = (ploidy ==
1)), get_locus_length_matrix(model), private$expand_mcmf, private$type_expand,
has_trios(model), ploidy) at /home/paul/coala/R/sumstat_mcmf.R:23
7. Error: mcmf can be calculated for all populations (@test-sumstat-mcmf.R#87) -
not compatible with requested type
1: expect_equal(stat$calculate(seg_sites, NULL, NULL, model1), stat$calculate(seg_sites,
NULL, NULL, model2)) at /home/paul/coala/tests/testthat/test-sumstat-mcmf.R:87
2: compare(object, expected, ...)
3: stat$calculate(seg_sites, NULL, NULL, model1)
4: calc_mcmf(seg_sites, get_population_individuals(model, private$population, haploids = (ploidy ==
1)), get_locus_length_matrix(model), private$expand_mcmf, private$type_expand,
has_trios(model), ploidy) at /home/paul/coala/R/sumstat_mcmf.R:23
I fear that I don't have to time to look into this at the moment. Can you try to fix the failures? You can run the tests by calling devtools::test()
.
Best, Paul
continued in #179
Add new features added to Coala's MCMF