Edge encoding

[jjustison]

See previous pull request here. PR'ing form this branch for more public access to modify code

[jjustison]

I just saw that the dev branch was made and had changes to it. I will merge these two branches and then perhaps it will be worthwhile to look more at this request

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
src/PhyloCoalSimulations.jl	100.00% <ø> (ø)
src/simulatecoalescent_network.jl	100.00% <100.00%> (ø)
src/simulatecoalescent_onepop.jl	100.00% <100.00%> (ø)
src/utils.jl	100.00% <100.00%> (ø)

[jjustison]

I was working on the pull request for PhyloCoalSimulations and ending up fixing some of the tests as part of the process. I have those changes in the edge_encoding branch where there is a pull request. This branch also currently has the most up to date changes merged from the dev
branch.

The only funky thing that I couldn't quite figure out was how to properly have SNaQ as a test-only dependency. I added the dev version of SNaQ as a dependency. It is only needed in the tests but whenever I try to remove it from [deps] in the Project file, I get the following error:
expected package SNaQ [c2bf7a07] to be registered.
Tests pass locally with how I have SNaQ set up but don't pass on CI.

[cecileane]

It would be so much better to not depend on SNaQ, even for tests.
What would you think of a small package for quartet concordance factor utilities, separate from SNaQ? I know it's more work, but I've come across this need multiple times now.

[jjustison]

I suppose it might depend on what all is included but if they are somewhat general utilities that come up in a few places, then perhaps they could go in PhyloNetworks? Otherwise, if it ends up being a large amount of utilities, and datatypes, then maybe a new package might make sense.

I don't take huge issue with depending on SNaQ for only tests but I could see it being unideal in other cases where packages have it as a dependency for only the few CF-related functions.

[cecileane]

I thought that having a separate project file test/Project.toml would make it easier to declare SNaQ as a dependency of running tests, but not a dependency of the package itself. I am trying strategies to limit the dependency of SNaQ to tests only. I hope to push an option soon.

[cecileane]

I hope all checks will pass, with SNaQ removed from the package's dependencies (but included for tests). The documentation should pass after PhyloPlots v2.0 is officially registered.

In my last commit, I exported the function population_mappedto and modified examples to promote the use of this function to access the embedding of gene trees into the species network, and to discourage the use of the internal field .inte1 (or whatever it is).

I wonder the new function encode_edges! could have a more explicit name, to refer to edges in gene trees (vs. species phylogeny) and to refer to a "mapping" or "embedding" inside species or populations, to make "encoding" more explicit. Perhaps embed_geneedges! ? or population_mapping! (to highlight the similarity to population_mappedto ? other ideas?

[cecileane]

I suppose it might depend on what all is included but if they are somewhat general utilities that come up in a few places, then perhaps they could go in PhyloNetworks? Otherwise, if it ends up being a large amount of utilities, and datatypes, then maybe a new package might make sense.

I don't take huge issue with depending on SNaQ for only tests but I could see it being unideal in other cases where packages have it as a dependency for only the few CF-related functions.

I agree. For now we can keep SNaQ as a dependency for tests. Independently, we can list the general utilities that come up in several places that don't need the snaq! estimation method. We've recently seeing many people and methods that can use quartet concordance factors as input, so utilities to read qCF data and calculate qCFs from gene trees would be part of this list of general utilities.

[jjustison]

I hope all checks will pass, with SNaQ removed from the package's dependencies (but included for tests). The documentation should pass after PhyloPlots v2.0 is officially registered.

In my last commit, I exported the function population_mappedto and modified examples to promote the use of this function to access the embedding of gene trees into the species network, and to discourage the use of the internal field .inte1 (or whatever it is).

I wonder the new function encode_edges! could have a more explicit name, to refer to edges in gene trees (vs. species phylogeny) and to refer to a "mapping" or "embedding" inside species or populations, to make "encoding" more explicit. Perhaps embed_geneedges! ? or population_mapping! (to highlight the similarity to population_mappedto ? other ideas?

I think perhaps we could combine both of those ideas into something like gene_edgemapping!. I think this combines the idea that it is something for the gene tree edges while also evoking similarity to population_mappedto. Otherwise I also just like embed_geneedges!, it is short and more explicit than encode_edges!

[cecileane]

Let's wait until PhyloPlots v2.0 is released, then re-run the documentation to make sure it builds. I don't understand what's holding back the release of PhyloPlots...