- 
                Notifications
    
You must be signed in to change notification settings  - Fork 58
 
Vector object automatically converts vectors to byte strings #411
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 PR automates the conversion of vector arrays to byte strings in the Vector object, simplifying the API by removing the need for manual conversion in MultiVectorQuery.
- Adds a model validator to 
Vectorclass that automatically converts float arrays to byte strings based on the specified dtype - Simplifies 
MultiVectorQuery.paramsby removing redundant conversion logic since vectors are now pre-converted - Adds comprehensive test coverage for both float-to-bytes conversion and direct bytes input scenarios
 
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description | 
|---|---|
| redisvl/query/aggregate.py | Added model validator to Vector class for automatic byte conversion; simplified MultiVectorQuery.params method | 
| tests/unit/test_aggregation_types.py | Added unit tests verifying Vector class handles both float arrays and byte strings correctly | 
| tests/integration/test_aggregation.py | Added integration test for MultiVectorQuery accepting pre-converted byte strings | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Looks good to me except for the unused import!
I am surprised that the linter tests are not catching this
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.