jguhlin / minimap2-rs

Rust bindings to minimap2 library
Other
60 stars 13 forks source link

Change to a more builder style syntax #3

Closed Adoni5 closed 1 year ago

Adoni5 commented 1 year ago

Hi Joseph,

I've made a small change to the syntax of initialising the Aligner, inspired by building a Server in the GRPC crate tonics style of building a server.

There's more about this in a blog post here - although this potentially a little bit extra!

The big difference is rather than instantiating a literal Struct -

Aligner {
     mapopt: MapOpt {
        seed: 42,
        best_n: 1,
        ..Default::default()
     },
    idxopt: IdxOpt {
        k: 21,
        ..Default::default()
    },
  ..map_ont()
};

You use a builder style API

Aligner::builder().map_ont().n_threads(1)... etc.

This involved moving all the ergonomic preset functions into the impl for Aligner, and changing all the doctests.

If you don't like it, no worries, I just find it lot more readable/writeable in this style!

I also changed the tests to use a yeast reference - although this was just included in this commit for no other reason than I forgot to change it back.

jguhlin commented 1 year ago

In my mind I wanted something like bevy's system, but I think with minimap2 everyone just uses presets. I'll get this integrated in over the next few days, I'm happy with finding a single style to go with. :)