Skip to content

Comments

feat!: streamline annotator interface + refactor internals#487

Closed
jsstevenson wants to merge 15 commits intomainfrom
annotator-structure
Closed

feat!: streamline annotator interface + refactor internals#487
jsstevenson wants to merge 15 commits intomainfrom
annotator-structure

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
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

Successfully merging this pull request may close these issues.

Refactor/clean up annotator module

1 participant