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

sets the default datatype in our vectorizers to float32 if not specified by users #253

Merged
merged 13 commits into from
Jan 8, 2025

Conversation

justin-cechmanek
Copy link
Collaborator

No description provided.

@justin-cechmanek justin-cechmanek marked this pull request as ready for review December 14, 2024 00:55
@tylerhutcherson tylerhutcherson force-pushed the feat/RAAE-517/default-float32 branch from 0ea192b to fb87288 Compare January 6, 2025 19:22
rbs333
rbs333 previously approved these changes Jan 6, 2025
Copy link
Collaborator

@rbs333 rbs333 left a comment

Choose a reason for hiding this comment

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

Few notes but nothing blocking

redisvl/utils/vectorize/base.py Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two small things on this

  1. this does have the warning which doesn't really matter but a heads up
image 2. is there a reason mistral is commented out? is there something we need to fix with that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah let's get rid of this warning. Good catch. 2 -- I think it had to do with stale API keys. @justin-cechmanek are those active again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like the Mistral client is deprecated. I'll look into updating our vectorizer class for it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice. I see this one here: #255

After we get this warning removed from the notebook, I am happy to merge!

@justin-cechmanek justin-cechmanek merged commit 6379951 into main Jan 8, 2025
20 checks passed
@justin-cechmanek justin-cechmanek deleted the feat/RAAE-517/default-float32 branch January 8, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants