Multiple Vector Support #14
Conversation
|
I think we need to remove lock file. Our releases are more stable and no need to keep it, too. I will do it in couple of minutes. |
|
We can rebase the branch to the main. I removed the |
|
|
||
| /** | ||
| * @var array|null Payload values (optional) | ||
| */ | ||
| protected ?array $payload = null; | ||
|
|
||
| public function __construct(string $id, array $vector, array $payload = null) | ||
| public function __construct(string $id, VectorStructInterface|array $vector, array $payload = null) |
There was a problem hiding this comment.
In my opinion, we can continue with an interface instead of an array, too.
There was a problem hiding this comment.
Do you mean leave it as is now, or should I remove support for arrays?
There was a problem hiding this comment.
I mean that we can continue with interface directly. Without array.
There was a problem hiding this comment.
It's been a few weeks, so just confirming that you'd like me to remove array as an option for $vector in this constructor?
It would be neater removing array, but for backwards compatibility it does help. We could also mark array for removal in a future version and just trigger a warning when an array is passed for $vector.
|
We need to add some tests. Also, here is the documentation for multiple vector support |
|
Hey guys, need a multiply vector support. Is it working with this realisation? |
|
I am in vacation. I will check again as soon as possible. @Nikcrysis |
|
Hello @gregpriday if you dont have time to finish maybe I can work on this PR. What do you think? |
hkulekci
force-pushed
the
feature/multiple-vector-support
branch
from
fd7ccdc
to
d469380
Compare
|
Okay PR rebased and I just push a change for PointsBatch request class. Please pull the changes on your local to test. Ping @gregpriday |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
============================================
+ Coverage 87.48% 87.60% +0.12%
- Complexity 263 278 +15
============================================
Files 56 57 +1
Lines 903 936 +33
============================================
+ Hits 790 820 +30
- Misses 113 116 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
@hkulekci Great, I'll start working on this again today. I have a few other PRs and suggestions coming too. |
|
Great! I just rebased the branch FYI. |
|
@hkulekci I've added some tests and made a few more changes. I can try to add a few more tests to hit the Codecov requirements. Is there anything else on my task list for this feature or are we ready to go once I had a few more tests? |
|
In fact, I am grateful for all this contributions. Thanks. I will do a last check after you finish all changes. Just change it as ready for merge whenever it is ready. |
hkulekci
force-pushed
the
feature/multiple-vector-support
branch
from
b74b0d9
to
0fbaf1b
Compare
Resolves #13
This is still a work in progress. Not happy with it yet, and I still need to write the tests, but putting this up in case you have any comments or would prefer a different direction.
One breaking change I made was using
VectorStructInterfacefor the$vectorargument of thePointconstructor. This makes a lot more sense to me.I've also structured the
MultiVectorStructin a way that makes it only compatible with Qdrant 1.2.