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

add custom builder for VectorsConfig #135

Merged
merged 7 commits into from
Jun 20, 2024
Merged

Conversation

generall
Copy link
Member

@generall generall commented Jun 20, 2024

Allows to create vector configs like this:

        let mut vector_config = VectorsConfigBuilder::default();

        vector_config.add_named_vector_params(
            "large_vector",
            VectorParamsBuilder::new(8, Distance::Cosine),
        );
        vector_config.add_named_vector_params(
            "small_vector",
            VectorParamsBuilder::new(4, Distance::Euclid),
        );

        let create_collection =
            CreateCollectionBuilder::new(collection_name).vectors_config(vector_config);

self
}

pub fn add_vector_params(&mut self, params: impl Into<VectorParams>) -> &mut Self {
Copy link
Contributor

@JojiiOfficial JojiiOfficial Jun 20, 2024

Choose a reason for hiding this comment

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

Not sure if add is the right prefix here. It sounds to me as if you can call it multiple times. What do you think about naming it with_vector_params or just vector_params as the other setter functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

then it won't be consistent with add_named_vector_params (which is not setter), because it won't overwrite data there with non-default name

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure whether it needs to be treated that similar to add_named_vector_params because one is basically a setter and one not.
It was just a suggestion because I can imagine user getting confused by this. With the comment I think it'd be fine either way so feel free to mark this as resolved

src/qdrant_client/builers/vectors_config.rs Outdated Show resolved Hide resolved
generall and others added 5 commits June 20, 2024 13:22
Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>
Co-authored-by: Tim Visée <tim@visee.me>
Co-authored-by: Tim Visée <tim@visee.me>
@generall generall merged commit 8cb21cc into v1-10-upgrade Jun 20, 2024
2 checks passed
timvisee added a commit that referenced this pull request Jun 26, 2024
* add custom builder for VectorsConfig

* fmt

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* fix clippy

* fix clippy

---------

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>
Co-authored-by: Tim Visée <tim@visee.me>
timvisee added a commit that referenced this pull request Jun 26, 2024
* add custom builder for VectorsConfig

* fmt

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* fix clippy

* fix clippy

---------

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>
Co-authored-by: Tim Visée <tim@visee.me>
timvisee added a commit that referenced this pull request Jun 27, 2024
* add custom builder for VectorsConfig

* fmt

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* Update src/qdrant_client/builers/vectors_config.rs

Co-authored-by: Tim Visée <tim@visee.me>

* fix clippy

* fix clippy

---------

Co-authored-by: Jojii <15957865+JojiiOfficial@users.noreply.github.com>
Co-authored-by: Tim Visée <tim@visee.me>
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