Make Vertex options hybrid

[philippgille]

As per #91 (comment), this PR is a suggestion to do kind of a hybrid for the options. We use mandatory parameters directly in the constructor function, but then instead of variadic functions we have one struct for optional configuration that can be adjusted with a fluent API (opts.With...().With...()).

Variadic functions are very popular and common in widely used libraries (e.g. grpc-go), and have the benefit of not requiring the last parameter in case you're fine with default values. Some downsides are that passing functions instead of a struct can be less intuitive (maybe just for Go newcomers), as well as that each With function for an optional option is then a global package member.

TBH I'm undecided which is best. This PR is just a suggestion and open for discussion.

The alternative is to revert to variadic functions, PR here: #93

[iwilltry42]

passing functions instead of a struct can be less intuitive (maybe just for Go newcomers)

As a non-newcomer, I still think it's not that intuitive. With a struct you can see right-away which options are available (the fields of that struct), even just by hovering over it in your IDE.
That kinda works for the builder pattern as well, because you see all the struct fields and can guess there's a function for each of the fields... or your IDE even shows the methods on the struct.

Variadic options always have to be looked up - but I like the function signature when using them, especially when adding them to an existing project where they won't break backwards compatibility.

My first take with the builder pattern was: "I'm going to pull the config definition out of the function call, because it's going to grow quite large and make it unreadable." - That's why I put the whole config into the config struct.
But I absolutely agree with you, that the mandatory options (that have no defaults) should be parameters themselves and not be part of the config struct.

Now this probably doesn't help you a lot other than saying, that both #92 and #93 are fine and better than my original implementation :)

[philippgille]

And while from an IDE perspective the options struct is better (immediately visible options), in the rendered Godoc it's the other way around:

With variadic functions all With functions are grouped with the option type:

And with the options struct, as long as it's not exported, it's not visible at all:

And if the options struct is exported, people may use that instead of DefaultVertexOptions() and then we'll have to check for if opts.apiEndpoint == "" to then set the default. And for boolean values, they must be either a pointer to know it was not just set to false because of the zero value, or named such that false is always the default option. Like when a provider usually returns normalized vectors, the option can't be bool normalize, but must be bool dontNormalize or similar. In another project I went this way, but it's not ideal.

Actually, as long as the options struct is not exported, it's not even as clear cut in the IDE:

• When having db.CreateCollection("knowledge-base", nil, chromem.NewEmbeddingFuncVertex("apikey", "proj", "model", chromem.DefaultVertexOptions())) in the code, you first have to follow DefaultVertexOptions() to be able to hover over the return value to see all fields of the struct.
• With variadic funcs, having db.CreateCollection("knowledge-base", nil, chromem.NewEmbeddingFuncVertex("apikey", "proj", "model", chromem.WithVertexAPIEndpoint("https://foo.com/v1"))) you can also follow WithVertexAPIEndpoint() and hovering over return func(o *vertexOptions) { also shows all fields.

One thing worth noting is that we already have Collection.QueryWithOptions since recently (not in a release yet), which makes use of an exported options struct. But it's a bit different in that it contains fields that have to be set, and which can't be moved out of the struct either as there are some "either or" options (e.g. either query by text and let chromem turn it into an embedding or query by embedding directly). QueryWithOptions was introduced to have the NegativeQuery option, without having to add it as parameter to both Collection.Query (query by text) and Collection.QueryEmbedding (query by embedding directly), making their parameter lists longer. And NegativeQuery requires options as well, so currently it's a nested struct. As it's not released yet, it's still a possibility to change that to have everything consistent.

Or maybe a breaking change is required anyway, if for example we want to remove the optional "where" and "whereDocument" filters from the Query parameter lists...

I'll think about everything a bit more. 🤔

[philippgille]

I chose the variadic functions for now, to have something to move on. I'm releasing a new version v0.7.0 of the library today, with the goal of having no breaking changes. Then for the next version there's already going to be a breaking change with #96, so I'm open to doing other breaking changes as well, including applying one of the options patterns to the existing Collection.Query functions (whose where and whereDocument params are optional and passing nil for them is not ideal), as well as open to redefining how we handle these options (variadic funcs, exported options struct, unexported options struct with constructor and builder functions).

[msoo14131]

F