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

Translator API/misc improvements #475

Open
5 tasks
jsstevenson opened this issue Dec 24, 2024 · 2 comments
Open
5 tasks

Translator API/misc improvements #475

jsstevenson opened this issue Dec 24, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@jsstevenson
Copy link
Contributor

A wishlist/brainstorm:

  • Richer documentation and type annotation. This module is serving a lot of roles right now and some internal design decisions have been made out of necessity for fulfilling those roles, and as personnel come and go/the years go on and on, we might lose sight of why those decisions were made. Stronger documentation about the why of some of these choices would help prevent breakage down the road (and might also resolve some of my thoughts/concerns below). Type annotation should be easier and would be a good short term step.
  • Constrain global fmt values in an enum
  • Document kwargs for translator methods directly in the implementation classes. Currently they're all located within the abstract parent, which means that docs for each implementation e.g. show the user some irrelevant kwargs.
  • Make documentation for individual _from_ methods accessible. Right now there's a docstring that refers a user to look at them for more information, but they're private (leading underscore), so the user technically isn't supposed to be able to/can't easily find them, either with help() or in docs if we ever make them.
  • Not clear to me why args in AlleleTranslator._create_allele are kwargs -- if it's for flexibility between individual _from_fmt methods, there are a few ways we could make valid args more explicit, either naming them all and giving them defaults, doing fmt-specific post-processing within the _from_fmt method, ...
@jsstevenson jsstevenson added the enhancement New feature or request label Dec 24, 2024
@korikuzma
Copy link
Contributor

This module is serving a lot of roles right now and some internal design decisions have been made out of necessity for fulfilling those roles, and as personnel come and go/the years go on and on, we might lose sight of why those decisions were made. Stronger documentation about the why of some of these choices would help prevent breakage down the road (and might also resolve some of my thoughts/concerns below).

There was a discussion on architectural decision records in a GKS meeting and I think this could help us.

@korikuzma
Copy link
Contributor

This issue title is specific to Translator module, but I think it could be applied throughout the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants