sourmash-bio / sourmash

Quickly search, compare, and analyze genomic and metagenomic data sets.
http://sourmash.readthedocs.io/en/latest/
Other
473 stars 80 forks source link

`index.RevIndex` is not a fully implemented `Index` class #1939

Open ctb opened 2 years ago

ctb commented 2 years ago

In https://github.com/sourmash-bio/sourmash/pull/1936, I'm adding tests for basic features of the Index class, and RevIndex doesn't pass them. (RevIndex is the rust-based reverse index added in https://github.com/sourmash-bio/sourmash/pull/1238.)

When uncommenting the build_revindex part of the index_obj fixture in tests/test_index_protocols.py from #1936, we see that select with the wrong ksize doesn't work:


self = <sourmash.index.revindex.RevIndex object at 0x7fd5d02b8cd0>, ksize = 21
moltype = None, kwargs = {}

    def select(self, ksize=None, moltype=None, **kwargs):
        if self.template:
            if ksize:
>               self.template.ksize = ksize
E               AttributeError: can't set attribute

src/sourmash/index/revindex.py:152: AttributeError

We might also want to do more specific tests of RevIndex in tests/test_index.py and/or add docs on its value, API, and intended use.

luizirber commented 2 years ago

I think select wasn't around when I started implementing RevIndex...

(there is a LOT of fixes to be done in RevIndex, I don't think it even uses prefetch, search and gather to their full extent)

ctb commented 2 years ago

sure, no huhu or urgency:). But once we have more thorough API tests as in #1936, we could use them to fix up RevIndex!