Description
I had a go in porting the new API to osrank, which result can be seen as part of this draft PR. @cloudhead raised some concerns on the design choice of having osrank_naive take a closure in the form of a to_annotation: &dyn Fn(&G::Node, Osrank) -> A::Annotation, wondering whether or not we could tuck this closure to be a method of the GraphAnnotator, and here I would like to argue that it's fairly tricky to do, hoping I can be proven wrong so that we can get rid of the closure, for which Alexis has an idiosyncrasy 
Why we need to_annotation in the first place? The reason is simple: we need to be able to "interpret" the concrete data in the graph and produce an annotation out of it, and we can do that only when we know which is the concrete graph we are dealing with. If we modify GraphAnnotator this way:
pub trait GraphAnnotator {
type Annotation;
/// Annotate the graph with some data. This is left intentionally abstract,
/// for the implementation to decide whether to expose a key/value like
/// interface with eg. `Annotation = (Key, Val)` or a batched interface
/// like `Annotation = Vec<(Key, Val)>`.
fn annotate_graph(&mut self, note: Self::Annotation);
fn to_annotation(&self, ???) -> Self::Annotation;
}What will be the type of ??? ? Even if we pass a Graph G as a type parameter, this doesn't solve the problem, because we need to interpret the individual node of a graph (for example, but this could be applied to anything, also edges, for example) in order to do something meaningful. Consider what we have now in osrank:
impl<'a> Default for OsrankNaiveMockContext<'a, MockAnnotator<MockNetwork>, MockNetwork> {
fn default() -> Self {
OsrankNaiveMockContext {
seed_set: None,
ledger_view: MockLedger::default(),
to_annotation: &mock_network_to_annotation,
}
}
}
fn mock_network_to_annotation(node: &Artifact<String>, rank: Osrank) -> (String, Osrank) {
let artifact_id = node.id().clone();
(artifact_id, rank)
}Here we are able to interpret the concrete node for our concrete mock implementation the way we like, and produce a (K,V) annotation out of it. Note how it is very convenient here to work with a richer Osrank type for testing, an ability we would like not to lose, ideally (making things monomorphic in f64 won't help too much anyway).
Thoughts? Ideas? Tales of glory?