Skip to content
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

feat!: streamline annotator interface + refactor internals #487

Closed
wants to merge 15 commits into from

Conversation

jsstevenson
Copy link
Contributor

@jsstevenson jsstevenson commented Feb 2, 2025

close #482

Breaking changes

  • make annotator submodule that houses cli and vcf modules. We've talked for a while about the naming incongruity between vcf_annotation.py and translator.py, so I resolved that + divided functionality out by CLI vs format-specific considerations.
  • streamlined data proxy construction for both the CLI and the annotator class. The Biocommons/GA4GH dataproxy already has a standardized URI format and a constructor method that lets you define this stuff in a single string, and from an interface perspective it's messy to have an option like "data proxy type" that then requires a different, additional option to be set depending on what you pick. This also makes the annotator more consistent with the translator interface.
    • This had a surprising effect on tests vis-a-vis cassettes. Previously, each test case was rebuilding the dataproxy instance because the annotator did this internally. However, by moving dataproxy construction out of the class and instead reusing the fixture in conftest.py, all tests after the first one stop sending requests to the dataproxy (i.e. they wouldn't trip any of the cassettes) because they are all LRU cache hits. I have a separate PR to alter the dataproxy fixture's scope, but that potentially comes with costs. As an intermediary measure, I made a separate case-scoped fixture just for the VCF tests module.
  • renamed a couple of arguments to the CLI and VCF annotator class to make them more consistent.
  • made the CLI args/options use hyphens instead of underscores. This probably doesn't matter but it's a bit more standard and I figured I was already breaking the interface.
  • pathlike inputs are Paths rather than strs
  • rename the custom exception to conform with https://docs.astral.sh/ruff/rules/error-suffix-on-exception-name/

Other notable changes

  • the VCF module was setting its own log level, fixed that

@jsstevenson
Copy link
Contributor Author

I think I've now put up PRs for everything that I'd originally done here, so I can close this

@jsstevenson jsstevenson closed this Feb 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor/clean up annotator module
1 participant