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

Establish an API for VariantLookup classes #88

Open
msto opened this issue Nov 14, 2024 · 0 comments
Open

Establish an API for VariantLookup classes #88

msto opened this issue Nov 14, 2024 · 0 comments
Assignees

Comments

@msto
Copy link
Collaborator

msto commented Nov 14, 2024

There has been lengthy discussion over what an appropriate API for the VariantLookup functionality should look like.

Currently, VariantLookup is an ABC with two concrete implementations - one for file-based lookups and one for cached, in-memory lookups.

An initial refactoring was begun in #62, to permit the user to interact with Primer3 without direct use of a VariantLookup.

In the course of #71, there was additional discussion over how we might want to expose (or hide) the various implementations.

An API that hides the details of implementation from the user might look as follows, making the current ABC and implementations hidden and storing the lookup as an attribute on a public class.

class VariantLookup:
    def __init__(...):
        ...
        self._lookup: _VariantLookup
        if cached:
            self._lookup = _InMemoryLookup()
        else:
            self._lookup = _DiskBasedLookup()
            
    def query(...):
        return self._lookup.query(...)

class _VariantLookup(ABC):
    ...

class _InMemoryLookup(_VariantLookup):
    ...

class _DiskBasedLookup(_VariantLookup):
    ...

Other points to take into consideration - we should be cautious of the constraints imposed by a common constructor since there will be some parameters specific to each concrete implementation1, and we should establish an initial set of parameters and methods that are universal (e.g. close())2.

Footnotes

  1. https://github.com/fulcrumgenomics/prymer/pull/71#discussion_r1805044436

  2. https://github.com/fulcrumgenomics/prymer/pull/71#pullrequestreview-2434670717

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