feature: allow 1D numpy array by tibor-reiss · Pull Request #1185 · weaviate/weaviate-python-client

tibor-reiss · 2024-07-18T19:01:49Z

This is the simplest fix which allows a 1d numpy array to be passed in (additionally, anything which can be converted in the function util.get_vector). This will raise a TypeError if the input is incorrect, which is converted later to WeaviateInvalidInputError.

Note that passing in something else then types.VECTOR, e.g. a numpy array, will raise a mypy error.

dirkkul · 2024-07-19T04:14:28Z

Hey, thank you for your contribution! Would you be able to add a test here: integration/test_batch_v4.py? Numpy is available as a test dependency

weaviate-git-bot · 2024-07-19T10:00:26Z

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

tibor-reiss · 2024-07-21T06:29:37Z

agree with the CLA

agree with the CLA

tibor-reiss · 2024-07-26T18:24:56Z

Hi @dirkkul, I think should be fine now. The two previous tests which failed (DB issue and timeout on grpc) were not connected to this PR.

tibor-reiss · 2024-09-11T19:42:39Z

@dirkkul friendly reminder: can this branch be merged?

dirkkul · 2024-09-13T07:06:07Z

integration/test_collection.py

 DATE3 = datetime.datetime.strptime("2019-06-10", "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc)


+def get_numpy_vector(input_list: list) -> Any:


we have numpy in our dev requirements, so you can assume it is present for tests

thanks, simplified
(and rebased - hopefully that fixes the failing test)

dirkkul · 2024-09-13T07:08:49Z

weaviate/collections/batch/grpc_batch_objects.py

                vector_bytes=(
                    pack_vector(obj.vector)
-                    if obj.vector is not None and isinstance(obj.vector, list)
+                    if obj.vector is not None and not isinstance(obj.vector, dict)


I think handling of vector input should be at the input level, eg in insert_many and batch.add_objects and not here

Could you please elaborate what you mean by this and some hints how to accomplish?

I was staring at the code and don't see how I could push the validation upstream, since obj.vector can be both None and a dict. Couple of lines below where vectors is constructed: if obj.vector is not None and isinstance(obj.vector, dict), which means either vector_bytes or vectors will be not-None, but not both at the same time.
For readability, we could move these two next to each other - just cosmetics.

Note: the only thing I would do is to move pack_vector outside as is _pack_named_vectors, and rename the latter to _pack_named_vector. But that is again just cosmetics :)

I think the correct place would be 'weaviate/collections/data/data.py::insert_manyand then the cleanup should be herevector=obj.vector`. But keep in mind that vectors can be lists and dictionaries (for named vectors) and both should be supported

Hi @dirkkul, thanks for coming back. However, I still don't see how this could be achieved without a major refactoring.

There are 3 possibilities in the current program flow, starting from data/_DataCollectionAsync.insert_many():

type(obj) type(obj.vector) _BatchObject.vector vector_bytes vectors

!DataObject n/a None None None

DataObject !dict obj.vector pack_vector(...) None

DataObject dict obj.vector None _pack_named_vectors(...)

So move one needs to move some of the logic from __grpc_objects() into insert_many(), e.g. creating a list of tuples containing (_BatchObject, vector_bytes, vectors). Is this what you are suggesting or am I on the wrong path please?

Update: the last commit contains a first try at refactoring. Let me know your thoughts please.

dirkkul · 2024-09-26T05:56:51Z

Hey, I think you are mixing two things up:

how the users supplies vectors
how we hand them internally (vector bytes, packing etc)

We would want that user supplied vectors that are numpy arrays are converted to python lists. So if you do

                _BatchObject(
                    collection=self.name,
                    vector=_get_vector_v4(obj.vector),
                    uuid=str(obj.uuid if obj.uuid is not None else uuid_package.uuid4()),
                    properties=cast(dict, obj.properties),
                    tenant=self._tenant,
                    references=obj.references,
                    index=idx,
                )

it should take care of things.

tibor-reiss · 2024-09-26T18:46:33Z

Hi @dirkkul, thanks for your patience, fingers crossed I got it now...

tibor-reiss · 2024-11-01T06:05:55Z

Friendly ping @dirkkul

tibor-reiss force-pushed the fix_1002_numpy_support branch from b196d53 to 02abfbd Compare July 25, 2024 20:20

tibor-reiss force-pushed the fix_1002_numpy_support branch from 802a74d to a2f116b Compare July 27, 2024 04:44

tibor-reiss force-pushed the fix_1002_numpy_support branch from a2f116b to 5120b1a Compare September 11, 2024 19:46

dirkkul reviewed Sep 13, 2024

View reviewed changes

tibor-reiss force-pushed the fix_1002_numpy_support branch from e36d66c to 3649fc5 Compare September 13, 2024 18:08

tibor-reiss added 5 commits September 25, 2024 20:00

Allow 1D numpy array

52ec9b3

Add tests

af1030b

Fix tests

fa9f776

Linting

1ec45b9

Simplify numpy usage

6eb0ad9

tibor-reiss force-pushed the fix_1002_numpy_support branch from 3649fc5 to 9f21009 Compare September 25, 2024 18:43

tibor-reiss requested a review from a team as a code owner September 25, 2024 19:47

tibor-reiss force-pushed the fix_1002_numpy_support branch from c564504 to e98b0aa Compare September 25, 2024 19:49

tibor-reiss requested a review from dirkkul September 25, 2024 19:50

tibor-reiss force-pushed the fix_1002_numpy_support branch 2 times, most recently from addbb98 to 00a60a8 Compare September 26, 2024 18:38

tibor-reiss added 2 commits September 26, 2024 20:40

Refactor

b1599a8

Reorg

27422ac

tibor-reiss force-pushed the fix_1002_numpy_support branch from 00a60a8 to 27422ac Compare September 26, 2024 18:40

		DATE3 = datetime.datetime.strptime("2019-06-10", "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc)


		def get_numpy_vector(input_list: list) -> Any:

type(obj)	type(obj.vector)	_BatchObject.vector	vector_bytes	vectors
!DataObject	n/a	None	None	None
DataObject	!dict	obj.vector	pack_vector(...)	None
DataObject	dict	obj.vector	None	_pack_named_vectors(...)

Conversation

tibor-reiss commented Jul 18, 2024

Uh oh!

dirkkul commented Jul 19, 2024

Uh oh!

weaviate-git-bot commented Jul 19, 2024

Uh oh!

tibor-reiss commented Jul 21, 2024

Uh oh!

tibor-reiss commented Jul 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tibor-reiss commented Sep 11, 2024

Uh oh!

dirkkul Sep 13, 2024

Choose a reason for hiding this comment

Uh oh!

tibor-reiss Sep 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dirkkul Sep 13, 2024

Choose a reason for hiding this comment

Uh oh!

tibor-reiss Sep 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dirkkul Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

tibor-reiss Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dirkkul commented Sep 26, 2024

Uh oh!

tibor-reiss commented Sep 26, 2024

Uh oh!

tibor-reiss commented Nov 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tibor-reiss commented Jul 26, 2024 •

edited

Loading

tibor-reiss Sep 13, 2024 •

edited

Loading

tibor-reiss Sep 13, 2024 •

edited

Loading

tibor-reiss Sep 25, 2024 •

edited

Loading