Simple aggregation redux

[afmagee42]

(This PR is mostly a synthesis of feedback from #11 and #12.)

This PR implements an interface for lineage aggregation, and some simple forms of lineage aggregation.

An Aggregator is an object that aggregates (via Aggregator.aggregate(input_taxa)) and produces an Aggregation.

An Aggregation is a subclassed dict[Taxon, Taxon] which gives a mapping from input/unaggregated taxa to some aggregated taxa. It does some limited correctness checking upon initialization and is capable of converting itself to a dict[str, str] (the anticipated use-case being by polars' or pandas' .replace() to aggregate line-list lineage data).

The user-facing Aggregators added are:

• BasicPhylogeneticAggregator, which uses a PhylogeneticTaxonomyScheme to perform taxonomic aggregation. This allows us to, for example, "group all children of LINEAGE.42 into LINEAGE.42," or "group all children of LINEAGE.42 into LINEAGE.42 except LINEAGE.42.47 and its children which are grouped into LINEAGE.42.47." The order of operations is defined by the ordering of the aggregation targets provided. The default assumption is if the user provides both LINEAGE.42.47 and LINEAGE.42, they intend a paraphyletic LINEAGE.42 (the "except" as above).
• ArbitraryAggregator, which enables the user to specify exactly the mapping they want for all taxa. (That is to say, functionally it does little.) This would, for example, enable grouping a set of COVID lineages into the non-phylogenetic units "SGTF" and "non-SGTF."
• SerialAggregator which takes in a set of aggregators and applies them in order. It is on the user to know whether this is a good idea or not. Putting an ArbitraryAggregator before a BasicPhylogeneticAggregator could easily lead to the BasicPhylogeneticAggregator doing nothing if none of the arbitrary taxa are found on the taxonomy tree.

New tests have been added for these aggregators via pytest.

The behind-the-scenes Aggregators are:

• SelfAggregator, which aggregates every taxon to itself.
• HomogenousAggregator, which aggregates every taxon into a single aggregation target.

BasicPhylogeneticAggregator Example

import cladecombiner

pn = cladecombiner.PangoSc2Nomenclature()

lineages = [
    "BA.2",
    "BA.2.86",
    "XCU",
    "XDQ",
    "BQ.1.1.23",
    "JN.6",
    "JN.1.9.1",
    "JN.9",
    "KP.1.1",
    "FW.1.1.1",
    "BA.5.2.6"
]

pn.setup_alias_map()

tree = pn.taxonomy_tree([cladecombiner.Taxon(lin, True) for lin in lineages])

scheme = cladecombiner.PhylogeneticTaxonomyScheme(tree)

input_taxa = [taxon for taxon in scheme.taxon_to_node.keys() if taxon.tip]

target_taxa = [
    cladecombiner.Taxon("BA.2", False), 
    cladecombiner.Taxon("BA.2.86", False),
    cladecombiner.Taxon("BA.5", False),
]

agg = cladecombiner.BasicPhylogeneticAggregator(target_taxa, scheme)

agg.aggregate(input_taxa)

Misc.

The handling of taxon ordering in BasicPhylogeneticAggregator is outsourced to the new sort_taxa() function, which sorts taxa such that larger taxa get larger indices. For example, we would sort such that we get [JN.1.1.1, JN.1.1, JN.1]. The ordering is not perfectly defined, e.g. either [JN.1.1.1, JN.1.1, JN.1.2, JN.1] or [JN.1.1.1, JN.1.2, JN.1.1, JN.1] would be equally valid.

A bug in PhylogeneticTaxonomyScheme.contains(x, y) was failing when x was the root, saying that the root did not contain any taxa. This has been fixed.

[swo]

This all seems reasonable to me. Huge improvement; lots of things are simplifed! Could be merged as-is, I think, although I haven't run the tests yet (on a plane!)

I have a bunch of little comments about architecture. Let me know what you think is in/out of scope and I can draft issues.

I think I'm still not entirely pleased with how composability (ie serial aggregation) works, but I don't know yet exactly how I think that should work in practice.

[swo]

I'm confused why we need both concepts "aggregation" and "taxon map"?

A taxon map could be a dictionary, which guarantees unique keys (the second of the two checks).

It feels like the first check (that the input taxa match the keys in the taxon map) is functionality that should live in the aggregator. The aggregator gets handed input taxa, and so it should check that it's outputting a taxon map that has all those input taxa as keys.

Right now it seems like the aggregator has to hand the input taxa and the taxon map to the aggregation. If the aggregator does one of those checks, then the taxon map can be just a map, with no further functionality?

[swo]

As per my comment above, I could see a static function that validates that an output taxon map has all the input taxa in it

Then you could count on code writers to know that their .aggregate() methods should call .validate_map() before returning the map.

[afmagee42]

I want some minimal validation automatically. Every taxon should be mapped exactly once is the minimum correctness standard, though some Aggregators might know more about what to check.

The choices seemed to be what I did (make the map check) or have Aggregator.aggregate() be a non-abstract method calling some actual aggregation function Aggregator._aggregate() and then Aggregator.validate(). Neither is ideal, but I'm open to being told one is less awkward than the other.

I'm also going to dispute the above "then the taxon map can be just a map, with no further functionality." Exporting the Taxon : Taxon dictto a str : str dict is going to be one of the most common usage patterns for the existence of this class, so it might as well be a method. Taxon objects are our internal representations, but everywhere else a user is going to need to interact with taxa, they'll be strings.

[swo]

Personally I would prefer just "PhylogeneticAggregator," because I can imagine some more "basic" forms, as per my comments about composability below

[afmagee42]

The name can change, but we have to be able to clearly distinguish between this implicitly fixed-map "put these in those" phylogenetic aggregation and the (probably multiple different) dynamic "put these in something" aggregator(s) that we want to make. My instinct is to qualify all of them (this one being "basic" or "fixed" or something of the sort), but I could be convinced that only the more complex ones need more explicit names.