amnh / PCG

𝙋𝙝𝙮𝙡𝙤𝙜𝙚𝙣𝙚𝙩𝙞𝙘 𝘾𝙤𝙢𝙥𝙤𝙣𝙚𝙣𝙩 𝙂𝙧𝙖𝙥𝙝 ⸺ Haskell program and libraries for general phylogenetic graph search
28 stars 1 forks source link

Refactor out Analysis module into a sub library #112

Closed recursion-ninja closed 5 years ago

recursion-ninja commented 5 years ago

The Analysis module, while a bit monolithic, is not a dependency of the Bio module. It can be safely moved to it's own sub library.

Once this is done, the unit test suites for this library will need to be moved as well. The integration tests can remain in the domain of the integration-tests executable.

Boarders commented 5 years ago

I tried to do this but Bio.Metadata.Dynamic.Class and Bio.Graph.ReferenceDAG.Internal currently depend upon Analysis (Analysis.Parsimony.Dynamic.DirectOptimization.Pairwise and Analysis.Parsimony.Internal respectively). Analysis.Parsimony.Internal is a standalone module to add type safety to node contexts and could easily be moved into the main library. The only thing that Bio.Metadata.Dynamic.Class depends on is DenseTransitionCostMatrix so it seems plausible that we could work around that also but that is code I haven't touched so I may need some input on the best place to put it.

recursion-ninja commented 5 years ago

I think we can move DenseTransitionCostMatrix to the pcg-tcm sub-library.

Boarders commented 5 years ago

Whilst doing this I also moved out TotalEdgeCost into the analysis submodule (though I believe we don't actually use it whatsoever currently). I also moved Test.Custom.DynamicCharacterNode into analysis for now but it might be worth removing this from our build as more often than not it is just a source of type errors.

recursion-ninja commented 5 years ago

TotalEdgeCost is a heuristic cost we'll need later. I think that Analysis is a fine place for it.

Agreed that something needs to be done regarding Test.Custom.DynamicCharacterNode.

Some executable in the utils folder depend on Test.Custom.DynamicCharacterNode. Maybe we should just pull the functionality out of that module and place it in the one or two Main modules in utils where it is needed?

Boarders commented 5 years ago

I think that is a good idea, I'll give it a go.

Boarders commented 5 years ago

The module and unit tests have now been factored out here: https://github.com/amnh/PCG/commit/31b8fef4704bac5116d946adfb05d681bd2ae532.