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

Where and how to define the data_proxy interface #390

Open
jsstevenson opened this issue Apr 10, 2024 · 1 comment
Open

Where and how to define the data_proxy interface #390

jsstevenson opened this issue Apr 10, 2024 · 1 comment
Labels
2.0-alpha Issues related to VRS 2.0-alpha branch technical debt A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup"

Comments

@jsstevenson
Copy link
Contributor

jsstevenson commented Apr 10, 2024

https://github.com/ga4gh/vrs-python/blob/main/src/ga4gh/vrs/dataproxy.py (always a tell when the code references "VR" and not "VRS")

Most of this is copy-pasted from SeqRepo. This seems less than ideal, but I think we probably didn't want to depend on SeqRepo for the dataproxy if it could be avoided.

  • Is that still necessary? Can't we install SeqRepo and just use some of the API library, not the actual data?
  • What about refget? It looks... a lot like the dataproxy API, and it's a GA4GH thing. Can we just define a refget API? Seqrepo-rest-server already provides refget output IIRC from what I hear refget is missing something that we expect from the dataproxy regarding alias lookup
  • Can we just define it once in a central location and have both SeqRepo and VRS Python import it?
@jsstevenson jsstevenson added 2.0-alpha Issues related to VRS 2.0-alpha branch priority:medium Medium priority technical debt A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup" and removed priority:medium Medium priority labels Apr 10, 2024
@jsstevenson
Copy link
Contributor Author

Didn't think much of it at the time, but the new derive_refget_accession method that was added to the VRS DataProxy class does mean you have to be careful about how you're constructing it in cases where you need a singleton instance shared across multiple resources (e.g. Cool Seq Tool, Variation Normalizer, etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-alpha Issues related to VRS 2.0-alpha branch technical debt A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup"
Projects
None yet
Development

No branches or pull requests

1 participant