Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Positioned trait may be more static for performance reason. #6

Open
38 opened this issue Jul 31, 2023 · 1 comment
Open

Positioned trait may be more static for performance reason. #6

38 opened this issue Jul 31, 2023 · 1 comment

Comments

@38
Copy link

38 commented Jul 31, 2023

pub trait Positioned {
    fn chrom(&self) -> &str;
    fn start(&self) -> u64;
    fn stop(&self) -> u64;

    // extract a value from the Positioned object Field
    fn value(&self, f: Field) -> Result<Value, FieldError>;
}

/// Value can be any number of Ints, Floats, or Strings.
pub enum Value {
    Ints(Vec<i64>),
    Floats(Vec<f64>),
    Strings(Vec<String>),
}

This Field type is a runtime polymorphism, which is not very desired for performance considerations. Actually you can make a faster version of the trait (and also more generic) with the following def.

pub trait Positioned<FieldType: Field> {
    fn chrom(&self) -> &str;
    fn start(&self) -> u64;
    fn stop(&self) -> u64;

    // extract a value from the Positioned object Field
    fn value(&self) -> Result<FieldType::Output, FieldError>;
}
trait Field {
    const NAME: &'static str;
    type Output;
}

And then you may have things like

struct Score;
impl Field For Score {
   const NAME: &'static str = "score";
   type Output = f64;
}

let score = Positioned::<Score>::value(&positioned);
@brentp
Copy link
Member

brentp commented Jul 31, 2023

Thanks Hao! I'm very glad to have your feedback! This is definitely at the bounds of my understanding.

With this setup, any Field would have to be known at compile time, right? I would like to support more dynamic field access as well.
How would this work with python bindings where I'd want the user to be able to choose arbitrary fields, like "INFO.AF" ?

I think that perhaps, the value method in the Positioned trait is not needed now that we have the Position enum. Instead of relying on value calls, we can instead discover the concrete type from the enum and then choose the appropriate means of getting information out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants