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

feat: enable logging in annotator CLI #450

Merged
merged 4 commits into from
Sep 11, 2024
Merged

feat: enable logging in annotator CLI #450

merged 4 commits into from
Sep 11, 2024

Conversation

jsstevenson
Copy link
Contributor

No description provided.

@jsstevenson jsstevenson added the priority:low Low priority label Sep 6, 2024
@jsstevenson jsstevenson requested review from a team as code owners September 6, 2024 17:22
@@ -36,10 +37,55 @@ class SeqRepoProxyType(str, Enum):
@click.group()
def _cli() -> None:
"""Annotate input files with VRS variation objects."""
logging.basicConfig(
filename="vrs-annotate.log",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made me realize that we should probably rename class / module to vrs-annotate if we plan on supporting more than just vcfs. Can create a new issue for this

Copy link
Contributor Author

@jsstevenson jsstevenson Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@korikuzma yeah I thought about doing that in the previous PR and wasn't sure about it, but would be happy to make a PR for it if we think it makes sense to do so now. At minimum I think it'd be nice to change "annotation" to "annotator" in the module name to match the tense of the translator module.

@jsstevenson jsstevenson merged commit 10157c3 into main Sep 11, 2024
6 checks passed
@jsstevenson jsstevenson deleted the vrs-annotate-log branch September 11, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Low priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants