-
Notifications
You must be signed in to change notification settings - Fork 46
Use new lazy_import function to lazily import third-party libraries #331
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
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.
Pull Request Overview
This pull request updates various modules to use the new lazy_import utility for third-party libraries, streamlining imports and potentially reducing startup overhead. Key changes include switching direct imports of numpy, nltk, tabulate, and ml_dtypes to lazy imports across multiple files and adjusting the test setup for bfloat16 handling.
- Migrate direct module imports to lazy_import where applicable
- Update various utility functions and CLI components to adopt lazy imports
- Adjust testing code to handle special cases for bfloat16 imports
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/unit/test_utils.py | Adjusted handling for bfloat16 testing with explicit ml_dtypes import |
redisvl/utils/optimize/utils.py | Replaced numpy import with lazy_import |
redisvl/utils/optimize/router.py | Replaced numpy import with lazy_import |
redisvl/utils/optimize/cache.py | Replaced numpy import with lazy_import |
redisvl/redis/utils.py | Migrated numpy and ml_dtypes import to lazy_import at module level, but retains explicit import for bfloat16 in utility functions |
redisvl/query/query.py & aggregate.py | Replaced nltk and its stopwords import with lazy_import |
redisvl/cli/stats.py & index.py | Updated tabulate import to lazy_import |
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.
LGTM beyond what the copilot is asking and my note about the helper function we have.
Uses the new lazy_import utility from: #330