Skip to content

"Table"-ness should be a trait? #21

New issue

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

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

Already on GitHub? # to your account

Closed
molpopgen opened this issue Jan 5, 2021 · 5 comments
Closed

"Table"-ness should be a trait? #21

molpopgen opened this issue Jan 5, 2021 · 5 comments

Comments

@molpopgen
Copy link
Member

molpopgen commented Jan 5, 2021

The "new" function, number of rows, and metadata are repeated for all the table types. What does it take to encapsulate these concepts into a trait?

@molpopgen molpopgen changed the title "Table"-ness should be a trait. "Table"-ness should be a trait? Mar 26, 2021
@molpopgen
Copy link
Member Author

The problem with making these a trait could be that traits are not accessible to downstream tools unless the trait is imported. So, to do this, we'd want to add some function to the table collection so that the user doesn't have to remember to import that.

@molpopgen
Copy link
Member Author

There are certainly come things "table-y" that can be traits. For example, row iteration:

pub trait TablesAPI<'a> {
    type Item: Sized;
    fn num_rows(&'a self) -> crate::tsk_size_t;
    fn iter(&'a self) -> Box<dyn Iterator<Item = Self::Item> + 'a>;
}

impl<'a> TablesAPI<'a> for crate::EdgeTable<'a> {
    type Item = EdgeTableRow;
    fn num_rows(&'a self) -> crate::tsk_size_t {
        self.num_rows()
    }
    fn iter(&'a self) -> Box<dyn Iterator<Item = Self::Item> + 'a> {
        let rv = self.iter();

        Box::new(rv)
    }
}

// Then, examples of generic functions using our new trait:

fn collect<'a, T: Sized>(t: &'a dyn TablesAPI<'a, Item = T>) -> Vec<T> {
    let mut rv = vec![];
    for x in t.iter() {
        rv.push(x);
    }
    rv
}

fn foo()
{
    use crate::TableAccess;
    let mut tables = crate::TableCollection::new(1.0).unwrap();
    let c = collect(&tables.edges());
}

The question is how much, if at all, this helps any client code?

@molpopgen
Copy link
Member Author

Another useful table-related trait would have the form:

pub trait MetadataDecoding {
    type MetadataType: MetadataRoundtrip;
    type IdType;

    fn decode(&self, row: Self::IdType) -> Option<Self::MetadataType>;
}

This trait uses associated types to remove a lot of the verbosity involved in decoding metadata.
The idea is that client code would define these trait impl for tables with metadata.
It'd be easy to provide a macro to build the impl for client code.

One design note is that a generic trait may be better, allowing the table collection/tree sequence to also play with these impl? This is something that needs thinking through and prototyping.

@molpopgen
Copy link
Member Author

Another useful table-related trait would have the form:

pub trait MetadataDecoding {
    type MetadataType: MetadataRoundtrip;
    type IdType;

    fn decode(&self, row: Self::IdType) -> Option<Self::MetadataType>;
}

This trait uses associated types to remove a lot of the verbosity involved in decoding metadata. The idea is that client code would define these trait impl for tables with metadata. It'd be easy to provide a macro to build the impl for client code.

One design note is that a generic trait may be better, allowing the table collection/tree sequence to also play with these impl? This is something that needs thinking through and prototyping.

Ideas related to this are happening in #251, which is likely to become an experimental feature set soon.

@molpopgen
Copy link
Member Author

Closed by #373

Not a trait, but a Deref target!

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

No branches or pull requests

1 participant