Unflatten index options consistently (+tests)#120
Merged
Conversation
Collaborator
Author
|
@a-random-steve FYI |
| /// <param name="sourceModel">Allows enabling certain vector optimizations on the index by specifying the source model for your vectors</param> | ||
| /// <returns></returns> | ||
| public TableVectorIndexDefinition Vector(SimilarityMetric? metric, string sourceModel) | ||
| public TableVectorIndexDefinition Vector(SimilarityMetric? metric, string? sourceModel) |
Contributor
There was a problem hiding this comment.
Due to support for .net 462, we've been trying not to use nullables on reference types in the actual API (except in tests). That is, I've been taking these warnings seriously and avoiding code that causes them.
|
|
||
| [JsonPropertyName("analyzer")] | ||
| [JsonConverter(typeof(ObjectOrStringConverter))] | ||
| public object Analyzer { get; set; } |
Contributor
There was a problem hiding this comment.
Part of infamous 99 was clearing all these warning from existing code :)
…builder; expand index tests
Collaborator
Author
|
@a-random-steve Thanks for spotting those! I should have addressed both (and while I was at it I added another set of xml comments to bring the warnings to zero. I know it's out of scope for this PR but it's just comments (ahem)). Checking the changes should be super quick (I am not so experienced with c# to trust myself with this, sorry). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #104 .
This PR replaces the manual un/nesting of the table index "Options" to and from the payloads and responses, restoring isomorphism between the class encapsulation and the shape of the JSONs.
The builder methods are essentially unchanged (and they are flat), only the data structures are brought in line with the underlying Data API structure.
(so, if you use a builder, it should stay unchanged; conversely, if you create the index definition manually, your code will change somewhat).
Along the way, some attention is given to the tests.
Default/omitted entries should now not make it into the payload.
Two more items on the "text index":
TextAnalyzerenum value, you'll read a string in both cases).