redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.06k stars 108 forks source link

Type Unions Broken #591

Closed hthall13 closed 2 months ago

hthall13 commented 5 months ago

HashModels don't appear to support pydantic Unions.

# test.py

from typing import Union
from redis_om import JsonModel, HashModel

class JModel(JsonModel):
    param: Union[str, int]

class HModel(HashModel):
    param: Union[str, int]

The issue prevents initialization of the class:

$ python3 test.py 
Traceback (most recent call last):
  File "/home/user/project/test.py", line 7, in <module>
    class HModel(HashModel):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1195, in __new__
    new_class = super().__new__(cls, name, bases, attrs, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/pydantic/v1/main.py", line 282, in __new__
    cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
  File "/usr/lib/python3.10/abc.py", line 106, in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1496, in __init_subclass__
    if issubclass(origin, typ):
  File "/usr/lib/python3.10/typing.py", line 1158, in __subclasscheck__
    return issubclass(cls, self.__origin__)
TypeError: issubclass() arg 1 must be a class

I get the same error when incorporating Unions in JsonModel Fields as well.

# test2.py

from typing import Union
from redis_om import JsonModel, Field

class JModel(JsonModel):
    param: Union[str, int] = Field(default = 0)

Produces:

$ python3 test2.py 
Traceback (most recent call last):
  File "/home/user/project/test2.py", line 4, in <module>
    class JModel(JsonModel):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1195, in __new__
    new_class = super().__new__(cls, name, bases, attrs, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/pydantic/v1/main.py", line 282, in __new__
    cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
  File "/usr/lib/python3.10/abc.py", line 106, in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1675, in __init_subclass__
    cls.redisearch_schema()
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1742, in redisearch_schema
    schema_parts = [schema_prefix] + cls.schema_for_fields()
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1753, in schema_for_fields
    cls.schema_for_type(json_path, name, "", _type, field.field_info)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1890, in schema_for_type
    elif any(issubclass(typ, t) for t in NUMERIC_TYPES):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1890, in <genexpr>
    elif any(issubclass(typ, t) for t in NUMERIC_TYPES):
TypeError: issubclass() arg 1 must be a class

However excluding the Field class seems to keep this from happening:

# working code

from typing import Union
from redis_om import JsonModel

class JModel(JsonModel):
    param: Union[str, int]

if __name__ == "__main__":
    model = JModel(param=0)

Is this related to some problem with later pydantic versions? It looks like there are at least 3 PRs waiting to be reviewed to solve that problem: #548, #556, #577

redis-om: 0.2.1 pydantic: 2.0 pydantic_core = 2.0.1

slorello89 commented 3 months ago

@hthall13 this will be somewhat relieved by #603 with a couple of caveats:

  1. Union's aren't really appropriate for indexing in Redis (it really needs to know the type ahead of time to make an intelligent choice as to how to index it)
  2. If you are using a hash model, Redis OM will not be able to properly rematerialize the int type (as it gets a string and it see's that a string is a valid type)

I've added some tests to #603 to test this case.