-
Notifications
You must be signed in to change notification settings - Fork 3
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
Display help menus more promptly #62
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the help menus appear much more quickly is certainly helpful for a user. But we should still consider a cost-benefit analysis.
Obviously, there is a benefit for the user. However, the benefit is only for learning how one can control what the tool does, not for running the tool for a chosen task.
In terms of cost, it's hard to see how there could be costs for the user, but there can be and arguably are costs for developers. For example, some loss of code clarity, which is perhaps minor. If, however, this change affects (reduces) the way code completion works in widely used code editors (because they can't determine the imports anymore), then I would consider this major. And at this point in the codebase's lifecycle I would rank developer efficiency higher than a user's ability to shave off 10 seconds from the time it takes to get a help message.
So how does this affect code completion in, say, VS Code?
I actually mostly meant code completions on the imported modules, not only the pybioclip modules. |
Not quite. But I notice that it's only the |
But your screenshot is with a direct import. Of course that works, that wasn't my concern. |
import os | ||
import json | ||
import sys | ||
import prettytable as pt | ||
import pandas as pd | ||
import argparse | ||
|
||
DEFAULT_MODEL_STR = "hf-hub:imageomics/bioclip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicates the value from here:
pybioclip/src/bioclip/predict.py
Line 19 in b3ac523
BIOCLIP_MODEL_STR = "hf-hub:imageomics/bioclip" |
|
||
|
||
class TestParser(unittest.TestCase): | ||
|
||
def test_parse_args_lazy_import(self): | ||
"""Test that Rank is only imported when needed""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of testing that certain classes are loaded or not could we test how long it takes to display help?
I think it would be rather easy to accidentally undo the changes here by importing something new in main.
I'm wondering if there is a way to break up
|
Addresses #57 (speed up help menu displays by deferring large imports until called for to execute a command).
For a brief demo of the quality of life improvement, timing runs on my laptop for displaying help menus before and after the lazy loading are shown below.
Eager loading, fresh environment:
Eager loading, subsequently in the same environment:
Lazy loading, fresh environment:
Lazy loading, subsequently in the same environment:
Similarly for sub-command menus.