redis vectorset embedding store #477
Conversation
|
@Jacopo47 Thanks for the great work! I'm wondering if we should use |
There was a problem hiding this comment.
@Jacopo47 Thank you!
Could you please add Builder pattern and some javadoc in the constructor and class to clarify the meaing of parameters in the constructor.
BTW, to make spotless happy, please run make format. Thank you!
|
|
||
| import java.util.UUID; | ||
|
|
||
| class RedisVectorSetsEmbeddingStoreRemovalIT extends EmbeddingStoreWithRemovalIT { |
There was a problem hiding this comment.
copy-paste issue in filename, should be RedisVectorSetsEmbeddingStoreRemovalIT.java
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
|
|
||
| public class RedisVectorSetsEmbeddingStoreIT implements AutoCloseable { |
There was a problem hiding this comment.
Please extend it with EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
| if (input == null) return null; | ||
|
|
||
| Function<Collection<?>, String> collectIntoArray = i -> { | ||
| List<String> values = i |
There was a problem hiding this comment.
nit: could be simplified with:
var joined = i
.stream()
.map(e -> {
if (e == null) {
return null;
} else if (e instanceof String s) {
return toString(s);
} else if (e instanceof UUID s){
return toString(s);
} else {
return e.toString();
}
})
.filter(Objects::nonNull)
.collect(Collectors.joining(","));| .flatMap(filterMapper::from); | ||
|
|
||
| if (filter.isPresent()) { | ||
| filter.ifPresent(params::filter); |
There was a problem hiding this comment.
duplicate filter.isPresent() check here.
| .stream() | ||
| .map(mapToEmbeddingMatch) | ||
| .filter(Optional::isPresent) | ||
| .map(Optional::orElseThrow) |
There was a problem hiding this comment.
I think throwing an exception when there are no matching elements seems a bit too harsh. Wouldn't it be better to just return an empty list?
Martin7-1
added
enhancement
|
Hi, @Martin7-1 |
2 participants
Issue
Closes #393
Change
General checklist
Checklist for adding new maven module
pom.xmlandlangchain4j-community-bom/pom.xmlChecklist for adding new embedding store integration
{NameOfIntegration}EmbeddingStoreITthat extends from eitherEmbeddingStoreITorEmbeddingStoreWithFilteringIT{NameOfIntegration}EmbeddingStoreRemovalITthat extends fromEmbeddingStoreWithRemovalITChecklist for changing existing embedding store integration
{NameOfIntegration}EmbeddingStoreworks correctly with the data persisted using the latest released version of LangChain4j