While working on this issue i notices that it makes it so that union types in the index schema are not caught.

So, the problem with having union types in the index schema is the following:

Say we have a schema like this:

class MyDoc(BaseDocument):
    tensor: Union[NdArray, str]
    
index = MyDocIndex[MyDoc](...)  # should fail

Then what what column type should the backend choose for this? It is not well defined, thus should not be allowed.

However, this would preclude pre-built docs to be used as schemas.
Note that union types can still be used as the schema of the data, so the following workaround would work:

class MyDoc(BaseDocument):
    tensor: Union[NdArray, str]
    
class MySchema(BaseDocument):
    tensor: NdArray
    
index = MyDocIndex[MySchema](...)  # works
index.index([MyDoc(tensor=np.random.rand(...)) for _ in range(10)])  # also works

So we have two options here, as I see it:

as suggested above, disallow all union types in the index schema, and raise an Exception. This would require the workaround shown above, even for built-in docs such as ImageDoc. It is the clearest "rule", but annoying to the user.
do this, but have an exception: Union types that only contain tensor-like stuff should be allowed, since they all map to a tensor type in the backend column anyways. So, Union[NdArray, TorchTensor] would be allowed in the index schema, while Union, NdArray, str] would not be. We even have a helper _is_tensor_union(..) somewhere for this. This would allow us to use pre-built docs like ImageDoc to be used as a schema, which would be nice; but it makes the "rules" a bit less clear.

Personally I think I would go for the second option, but maybe as a first iteration the the first option works. @AnneYang720 up to you!

DocIndex: Fix flatten_schema #1288

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions